fix(claude-ops): harden lanes launcher refresh — abort on failure, validate targets first

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",

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

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 <name> <model> <effort> <prompt_path>` 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

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" <<STUB
 #!/usr/bin/env bash
 printf 'git %s\n' "\$*" >>"$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