diff --git a/plugins/claude-ops/.claude-plugin/plugin.json b/plugins/claude-ops/.claude-plugin/plugin.json index 2a3d369b4..c60f9642e 100644 --- a/plugins/claude-ops/.claude-plugin/plugin.json +++ b/plugins/claude-ops/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "claude-ops", - "version": "0.15.0", + "version": "0.15.1", "description": "Claude Code operations toolkit. Seven skills: observability (read locally captured telemetry — OTEL store, collector, hook-event JSONL, ccusage — with trend reports and store pruning), known-issues (search known Claude product GitHub bugs, check service health, maintain a persistent tracked-issue registry), changelog (ingest Claude Code changelog entries and integrate them into the current repo), plugins (bring a machine's plugin fleet current on demand — marketplace refresh, effective-scope updates including in-repo project/local installs, new-plugin install per policy, scope-divergence detection and explicit convergence), morning-brief (read-only gh-based operator morning view — queue-label counts, merge-ready PRs, parked decisions with their RECOMMENDED lines, and loop-lane telemetry freshness), lanes (start/restart/stop/status loop lanes as named background Claude Code sessions seeded from canonical prompt files, with per-lane model/effort and a repo-pull + marketplace-refresh launch step), and a re-runnable setup action that settles where the known-issues registry lives. Plus a family of seven advisory *-audit telemetry-emitter hooks (API errors, config changes, instruction loads, permission denials, pre-compaction, skill usage, tool failures) that emit the shared hook-telemetry envelope, and a reference sink that maps envelopes into the hook-events.jsonl the observability skill reads.", "author": { "name": "Melodic Software", diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index 65ca6eaa0..9fb7f207c 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -3,6 +3,26 @@ All notable changes to the `claude-ops` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.15.1] + +### Fixed + +- **`lanes` skill — launch aborts on a failed pre-launch refresh.** `start` / + `restart` previously ran `refresh_repo_and_plugins || rc=1` and launched lanes + regardless, so a failed `git pull --ff-only` (divergent/dirty checkout) or + `claude plugin marketplace update` still seeded background lanes from stale + repo/plugin state. The refresh is a documented launch prerequisite, so an + unexpected failure now hard-stops the launch (exit non-zero) with an actionable + message; `--no-pull` / `--no-update` remain the intentional-skip path (a + skipped step is not a failure). (#639) + +- **`lanes` skill — unknown restart/stop targets are rejected before any refresh + mutation.** `restart does-not-exist` ran `git pull --ff-only` + + `claude plugin marketplace update` before discovering the target was unknown. + The `TARGET_LANES` existence check now runs up front in `main`, ahead of the + refresh step, so a misspelled target fails fast (exit 3) with no repo/plugin + mutation — matching `stop`'s fail-first behaviour. (#639) + ## [0.15.0] ### Added diff --git a/plugins/claude-ops/skills/lanes/scripts/lane-launcher.sh b/plugins/claude-ops/skills/lanes/scripts/lane-launcher.sh index 8979fbd56..3f244f732 100755 --- a/plugins/claude-ops/skills/lanes/scripts/lane-launcher.sh +++ b/plugins/claude-ops/skills/lanes/scripts/lane-launcher.sh @@ -385,30 +385,40 @@ refresh_repo_and_plugins() { info "claude plugin marketplace update" run claude plugin marketplace update || rc=1 fi + # A skipped step (--no-pull/--no-update) leaves rc=0, so this only fires on an + # UNEXPECTED failure of a step that actually ran — the intentional-bypass path + # stays clean. Callers abort the launch on non-zero (see action_start/restart). + ((rc)) && err "refresh failed (pass --no-pull/--no-update to skip refresh intentionally)" return "$rc" } +# --- Target validation -------------------------------------------------------- +# Reject any explicit TARGET_LANES name absent from the config. Called from main +# ahead of action dispatch so an invalid target fails fast (exit 3) BEFORE any +# refresh/mutation runs — matching stop's fail-first DX rather than pulling the +# repo and updating plugins only to reject a misspelled target afterward. +validate_target_lanes() { + ((${#TARGET_LANES[@]})) || return 0 + local t known + for t in "${TARGET_LANES[@]}"; do + known="$(jq -r --arg n "$t" '[.lanes[].name] | index($n) // "no"' "$CONFIG")" + [[ "$known" != "no" ]] || { + err "unknown lane '$t' (not in $CONFIG)" + exit 3 + } + done +} + # --- Lane iteration helper ---------------------------------------------------- # Runs `callback ` for every lane, or only -# the lanes named in TARGET_LANES. Unknown target names are an error. +# the lanes named in TARGET_LANES. Target names are validated up front by +# validate_target_lanes (called from main), so every name here is already known. for_each_lane() { local callback="$1" pdir pdir="$(resolve_prompt_dir)" local count count="$(jq -r '.lanes | length' "$CONFIG")" - # Validate any explicit targets against the config first. - if ((${#TARGET_LANES[@]})); then - local t known - for t in "${TARGET_LANES[@]}"; do - known="$(jq -r --arg n "$t" '[.lanes[].name] | index($n) // "no"' "$CONFIG")" - [[ "$known" != "no" ]] || { - err "unknown lane '$t' (not in $CONFIG)" - exit 3 - } - done - fi - local i name model effort prompt_path failures=0 for ((i = 0; i < count; i++)); do name="$(lane_field "$i" name)" @@ -488,7 +498,10 @@ lane_scope() { ((${#TARGET_LANES[@]})) && printf ' (%s)' "${TARGET_LANES[*]}"; } action_start() { local rc=0 info "== lanes: start ==" - refresh_repo_and_plugins || rc=1 + # A failed refresh aborts BEFORE launching: never seed lanes from stale + # repo/plugin state the user did not sign off on (--no-pull/--no-update is the + # intentional-skip path, which leaves the refresh status 0). + refresh_repo_and_plugins || return 1 info "lanes:" for_each_lane _start_one || rc=1 return "$rc" @@ -497,7 +510,7 @@ action_start() { action_restart() { local rc=0 info "== lanes: restart$(lane_scope) ==" - refresh_repo_and_plugins || rc=1 + refresh_repo_and_plugins || return 1 info "lanes:" for_each_lane _restart_one || rc=1 return "$rc" @@ -521,6 +534,9 @@ main() { require_claude resolve_repo resolve_config + # Validate explicit targets up front — before session load and, crucially, + # before any action's refresh step mutates the repo/plugin state (Item 2). + validate_target_lanes [[ -z "$AGENTS_JSON_FILE" || -f "$AGENTS_JSON_FILE" ]] || { err "agents-json file not found: $AGENTS_JSON_FILE" exit 4 diff --git a/plugins/claude-ops/skills/lanes/scripts/lane-launcher.test.sh b/plugins/claude-ops/skills/lanes/scripts/lane-launcher.test.sh index fe573b069..6c22aaa7f 100755 --- a/plugins/claude-ops/skills/lanes/scripts/lane-launcher.test.sh +++ b/plugins/claude-ops/skills/lanes/scripts/lane-launcher.test.sh @@ -97,10 +97,17 @@ fi # A test can force a failed \`claude stop\` via STUB_CLAUDE_STOP_RC to exercise # the stop-failure paths (no relaunch, non-zero exit). if [[ "\$1" == "stop" ]]; then exit "\${STUB_CLAUDE_STOP_RC:-0}"; fi +# A test can force a failed \`claude plugin marketplace update\` via +# STUB_CLAUDE_UPDATE_RC to exercise the refresh-failure abort path. +if [[ "\$1" == "plugin" ]]; then exit "\${STUB_CLAUDE_UPDATE_RC:-0}"; fi STUB cat >"$STUB_BIN/git" <>"$CLAUDE_LOG" +# A test can force a failed \`git pull\` via STUB_GIT_PULL_RC to exercise the +# refresh-failure abort path. git is only ever invoked for pull here (repos are +# passed via --repo), so matching on the pull subcommand is sufficient. +case "\$*" in *pull*) exit "\${STUB_GIT_PULL_RC:-0}" ;; esac STUB chmod +x "$STUB_BIN/claude" "$STUB_BIN/git" @@ -341,6 +348,68 @@ out="$(STUB_CLAUDE_AGENTS_RC=1 run_launcher start --repo "$REPO" --config "$CONF rc=$? assert_eq "dry-run tolerates a failed live list (exit 0)" 0 "$rc" +# ============================================================================ +# A failed pre-launch refresh ABORTS the launch: no lane is started/stopped and +# the action exits non-zero with an actionable message. The refresh is a +# documented launch prerequisite, so stale repo/plugin state must never seed +# background lanes. +# ============================================================================ +: >"$CLAUDE_LOG" +out="$(STUB_GIT_PULL_RC=1 run_launcher start --repo "$REPO" --config "$CONFIG" --agents-json "$AGENTS_EMPTY" 2>&1)" +rc=$? +log="$(cat "$CLAUDE_LOG")" +assert_eq "start aborts when git pull fails (exit 1)" 1 "$rc" +assert_contains "start refresh-failure message is actionable" "$out" "refresh failed" +assert_not_contains "no lane launched after a failed pull" "$log" "--bg -n" + +: >"$CLAUDE_LOG" +out="$(STUB_CLAUDE_UPDATE_RC=1 run_launcher start --repo "$REPO" --config "$CONFIG" --agents-json "$AGENTS_EMPTY" 2>&1)" +rc=$? +log="$(cat "$CLAUDE_LOG")" +assert_eq "start aborts when marketplace update fails (exit 1)" 1 "$rc" +assert_contains "start marketplace-update refresh-failure message is actionable" "$out" "refresh failed" +assert_not_contains "no lane launched after a failed update" "$log" "--bg -n" + +: >"$CLAUDE_LOG" +out="$(STUB_GIT_PULL_RC=1 run_launcher restart work --repo "$REPO" --config "$CONFIG" --agents-json "$AGENTS_RUNNING" 2>&1)" +rc=$? +log="$(cat "$CLAUDE_LOG")" +assert_eq "restart aborts when git pull fails (exit 1)" 1 "$rc" +assert_not_contains "restart does not stop a lane after a failed pull" "$log" "stop sid-work-1" +assert_not_contains "restart does not relaunch after a failed pull" "$log" "--bg -n" + +# The intentional-bypass path (--no-pull/--no-update) is NOT a refresh failure: +# the skipped-step status stays 0, so lanes still launch even with the failure +# env set (proves the abort keys on real failure, not on skipping). +out="$(STUB_GIT_PULL_RC=1 STUB_CLAUDE_UPDATE_RC=1 run_launcher start --repo "$REPO" --config "$CONFIG" --agents-json "$AGENTS_EMPTY" --no-pull --no-update --dry-run 2>&1)" +rc=$? +assert_eq "refresh bypass still launches despite failure env (exit 0)" 0 "$rc" +assert_contains "refresh bypass launches lanes" "$out" "claude --bg -n work" + +# The same intentional-bypass invariant holds for restart, which refreshes via a +# different callback path (_restart_one): --no-pull/--no-update keeps the refresh +# status 0, so a targeted dry-run restart still previews the relaunch despite the +# failure env being set. +out="$(STUB_GIT_PULL_RC=1 STUB_CLAUDE_UPDATE_RC=1 run_launcher restart work --repo "$REPO" --config "$CONFIG" --agents-json "$AGENTS_RUNNING" --no-pull --no-update --dry-run 2>&1)" +rc=$? +assert_eq "restart refresh bypass still relaunches despite failure env (exit 0)" 0 "$rc" +assert_contains "restart refresh bypass previews the lane relaunch" "$out" "claude --bg -n work" + +# ============================================================================ +# An unknown restart target is rejected BEFORE the refresh mutates anything +# (matches stop's fail-first behaviour): the log shows no git pull and no +# marketplace update. Regression guard against the old order that pulled + +# updated only to reject the misspelled target afterward. +# ============================================================================ +: >"$CLAUDE_LOG" +out="$(run_launcher restart does-not-exist --repo "$REPO" --config "$CONFIG" --agents-json "$AGENTS_RUNNING" 2>&1)" +rc=$? +log="$(cat "$CLAUDE_LOG")" +assert_eq "restart unknown lane exits 3" 3 "$rc" +assert_contains "restart unknown lane message" "$out" "unknown lane 'does-not-exist'" +assert_not_contains "restart unknown lane does not pull" "$log" "pull --ff-only" +assert_not_contains "restart unknown lane does not update the marketplace" "$log" "plugin marketplace update" + # ============================================================================ echo if ((FAILED)); then