Browse Source

fix(git-ops): harden new-lane + worktree-guard edges; add test suite

Found via an adversarial e2e pass; all fixes pinned by a new 33-assertion
offline suite (skills/git-ops/tests/run.sh, auto-run by tests/run-skill-tests.sh):

- new-lane.sh: reject unknown -* options — a typo'd flag previously became the
  slug, yielding a cryptic git exit 128; now a clean exit 2. Validate the base
  ref and branch/path conflicts BEFORE any mutation, so a bad base or a
  collision can no longer leave a stray .gitignore append (partial state).
- worktree-guard.sh: count git clean's true force LEVEL (--force occurrences
  plus f's in short bundles) instead of only matching the -ff bundle — so
  -f -f, -fd -fx and --force --force (all force>=2, all delete nested
  worktrees) are caught, while a safe single-force -fdx stays silent.

E2e verified: an in-repo lane from new-lane.sh tracks + lands onto main via
fleet-ops (lane/<slug> through the %2F-encoded lane file).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0xDarkMatter 2 weeks ago
parent
commit
efb83b54f4
3 changed files with 138 additions and 10 deletions
  1. 12 2
      hooks/worktree-guard.sh
  2. 15 8
      skills/git-ops/scripts/new-lane.sh
  3. 111 0
      skills/git-ops/tests/run.sh

+ 12 - 2
hooks/worktree-guard.sh

@@ -71,9 +71,19 @@ elif printf '%s' "$CMD" | grep -qE '\bgit\b.*\badd[[:space:]]+([^;|&]*[[:space:]
      && [[ -d "$CWD/.claude/worktrees" ]]; then
      && [[ -d "$CWD/.claude/worktrees" ]]; then
   VIOLATION="git add -A/. in a repo with .claude/worktrees (may stage worktree gitlinks)"
   VIOLATION="git add -A/. in a repo with .claude/worktrees (may stage worktree gitlinks)"
 elif printf '%s' "$CMD" | grep -qE '\bgit\b[^;|&]*\bclean\b' \
 elif printf '%s' "$CMD" | grep -qE '\bgit\b[^;|&]*\bclean\b' \
-     && printf '%s' "$CMD" | grep -qE '(-[a-eg-z]*f[a-eg-z]*f|--force[^;|&]*--force)' \
      && [[ -d "$CWD/.claude/worktrees" ]]; then
      && [[ -d "$CWD/.claude/worktrees" ]]; then
-  VIOLATION="git clean -ff (double-force) in a repo with .claude/worktrees (force-removes live lanes + their uncommitted work)"
+  # git deletes a NESTED worktree only at force level >= 2 (a single -f safely
+  # "Skipping repository …"). Force level = count of --force PLUS the count of
+  # 'f' chars inside short bundles, so -ff, -f -f, -fd -fx and --force --force
+  # all read as 2, while a safe -fdx reads as 1. Bound to the clean invocation.
+  SEG=$(printf '%s' "$CMD" | grep -oE '\bclean\b[^;|&]*' | head -1)
+  # NB: count OCCURRENCES (grep -o | wc -l), not matching lines (grep -c counts
+  # lines, so two --force on one line would wrongly read as 1).
+  NLONG=$(printf '%s' "$SEG" | grep -oE -- '--force' | wc -l | tr -d ' ')
+  NSHORT=$(printf '%s' "$SEG" | grep -oE -- '(^|[[:space:]])-[A-Za-z]+' | grep -v -- '--' | tr -cd 'fF' | wc -c | tr -d ' ')
+  if [[ $(( ${NLONG:-0} + ${NSHORT:-0} )) -ge 2 ]]; then
+    VIOLATION="git clean double-force in a repo with .claude/worktrees (force-removes live lanes + their uncommitted work)"
+  fi
 fi
 fi
 
 
 [[ -z "$VIOLATION" ]] && exit 0   # clean → silent
 [[ -z "$VIOLATION" ]] && exit 0   # clean → silent

+ 15 - 8
skills/git-ops/scripts/new-lane.sh

@@ -36,6 +36,7 @@ for a in "$@"; do
     --sibling) SIBLING=1 ;;
     --sibling) SIBLING=1 ;;
     -h|--help) show_help; exit 0 ;;
     -h|--help) show_help; exit 0 ;;
     --) : ;;
     --) : ;;
+    -*) echo "new-lane: unknown option '$a' (valid: --sibling, -h/--help)" >&2; exit 2 ;;
     *) ARGS+=("$a") ;;
     *) ARGS+=("$a") ;;
   esac
   esac
 done
 done
@@ -61,23 +62,29 @@ REPO=$(basename "$MAIN")
 BRANCH="lane/$SLUG"
 BRANCH="lane/$SLUG"
 BASE="${2:-$(git -C "$MAIN" rev-parse --abbrev-ref HEAD 2>/dev/null)}"
 BASE="${2:-$(git -C "$MAIN" rev-parse --abbrev-ref HEAD 2>/dev/null)}"
 
 
+# Resolve the lane path (no side effects yet).
 if [ "$SIBLING" -eq 1 ]; then
 if [ "$SIBLING" -eq 1 ]; then
   WT="$(dirname "$MAIN")/${REPO}-${SLUG}"
   WT="$(dirname "$MAIN")/${REPO}-${SLUG}"
 else
 else
-  # In-repo placement is only safe when the lane dir is gitignored — otherwise
-  # `git add -A` from the main checkout stages worktree gitlinks (the 2026-04-19
-  # incident). Ensure the ignore so the in-repo default is safe in ANY repo, not
-  # just one someone already tidied.
-  if ! git -C "$MAIN" check-ignore -q ".claude/worktrees/.probe" 2>/dev/null; then
-    printf '\n# git worktrees / lanes — never tracked (new-lane.sh)\n.claude/worktrees/\n' >> "$MAIN/.gitignore"
-    echo "new-lane: '.claude/worktrees/' was not gitignored — added it to .gitignore (commit this)" >&2
-  fi
   WT="$MAIN/.claude/worktrees/$SLUG"
   WT="$MAIN/.claude/worktrees/$SLUG"
 fi
 fi
 
 
+# Validate EVERYTHING before mutating anything — a bad base or a conflict must
+# never leave partial state (e.g. a stray .gitignore append, see below).
+git -C "$MAIN" rev-parse --verify --quiet "${BASE}^{commit}" >/dev/null 2>&1 \
+  || { echo "new-lane: base '$BASE' is not a valid branch/commit" >&2; exit 2; }
 git -C "$MAIN" show-ref --verify --quiet "refs/heads/$BRANCH" && { echo "new-lane: branch $BRANCH already exists" >&2; exit 1; }
 git -C "$MAIN" show-ref --verify --quiet "refs/heads/$BRANCH" && { echo "new-lane: branch $BRANCH already exists" >&2; exit 1; }
 [ -e "$WT" ] && { echo "new-lane: path $WT already exists" >&2; exit 1; }
 [ -e "$WT" ] && { echo "new-lane: path $WT already exists" >&2; exit 1; }
 
 
+# In-repo placement is only safe when the lane dir is gitignored — otherwise
+# `git add -A` from the main checkout stages worktree gitlinks (the 2026-04-19
+# incident). Ensure the ignore now that all checks have passed, so the in-repo
+# default is safe in ANY repo — not just one someone already tidied.
+if [ "$SIBLING" -ne 1 ] && ! git -C "$MAIN" check-ignore -q ".claude/worktrees/.probe" 2>/dev/null; then
+  printf '\n# git worktrees / lanes — never tracked (new-lane.sh)\n.claude/worktrees/\n' >> "$MAIN/.gitignore"
+  echo "new-lane: '.claude/worktrees/' was not gitignored — added it to .gitignore (commit this)" >&2
+fi
+
 git -C "$MAIN" worktree add "$WT" -b "$BRANCH" "$BASE" >&2
 git -C "$MAIN" worktree add "$WT" -b "$BRANCH" "$BASE" >&2
 
 
 # Carry over gitignored env/secret files the new worktree won't have (Cloudflare/Node/etc.).
 # Carry over gitignored env/secret files the new worktree won't have (Cloudflare/Node/etc.).

+ 111 - 0
skills/git-ops/tests/run.sh

@@ -0,0 +1,111 @@
+#!/usr/bin/env bash
+# Self-test for git-ops worktree provisioning. Offline + deterministic (git only,
+# no network). Covers new-lane.sh (in-repo default, gitignore precondition,
+# --sibling, main-anchoring, validate-before-mutate, arg hygiene) and the
+# worktree-guard.sh `git clean` double-force detection.
+# Resolves paths relative to itself so it runs in the repo and once installed.
+#
+# Usage:   bash tests/run.sh
+# Exit:    0 all pass, 1 one or more failures (SKIP+exit 0 if git is unavailable)
+set -uo pipefail
+
+HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+SKILL="$(dirname "$HERE")"
+NL="$SKILL/scripts/new-lane.sh"
+# worktree-guard.sh lives in the repo's hooks/ — resolve for both repo + installed layouts.
+GUARD=""
+for c in "$SKILL/../../hooks/worktree-guard.sh" "$HOME/.claude/hooks/worktree-guard.sh"; do
+  [ -f "$c" ] && { GUARD="$c"; break; }
+done
+
+command -v git >/dev/null 2>&1 || { echo "SKIP: git not available"; exit 0; }
+
+SB="$(mktemp -d)"; trap 'rm -rf "$SB"' EXIT
+P=0; F=0
+ok(){ P=$((P+1)); printf '  PASS  %s\n' "$1"; }
+no(){ F=$((F+1)); printf '  FAIL  %s\n' "$1"; }
+mkrepo(){ rm -rf "$1"; git init -q -b main "$1"; git -C "$1" config user.email t@t
+  git -C "$1" config user.name t; git -C "$1" config core.autocrlf false
+  echo base > "$1/f"; git -C "$1" add -A; git -C "$1" commit -qm init; }
+
+echo "=== git-ops new-lane / worktree-guard self-test ==="
+
+echo "-- new-lane: in-repo default + gitignore precondition --"
+mkrepo "$SB/A"; cd "$SB/A"
+OUT="$(bash "$NL" auth-refactor 2>/dev/null)"; rc=$?
+[ $rc -eq 0 ] && ok "exit 0" || no "exit $rc"
+case "$OUT" in */.claude/worktrees/auth-refactor) ok "stdout = in-repo path only";; *) no "stdout=[$OUT]";; esac
+[ -d "$SB/A/.claude/worktrees/auth-refactor" ] && ok "lane dir created in-repo" || no "lane dir missing"
+git check-ignore -q .claude/worktrees/.x && ok ".claude/worktrees/ now gitignored" || no "gitignore not ensured"
+git show-ref --verify --quiet refs/heads/lane/auth-refactor && ok "branch lane/auth-refactor" || no "branch missing"
+
+echo "-- new-lane: no duplicate gitignore on second lane --"
+before=$(grep -c worktrees .gitignore); bash "$NL" hotfix main >/dev/null 2>&1
+[ "$before" = "$(grep -c worktrees .gitignore)" ] && ok "gitignore not duplicated" || no "gitignore duplicated"
+
+echo "-- new-lane: blanket .claude/ ignore is respected (no append) --"
+mkrepo "$SB/B"; cd "$SB/B"; printf '.claude/\n' > .gitignore; git add .gitignore; git commit -qm ig
+sz=$(wc -c <.gitignore); bash "$NL" x >/dev/null 2>&1
+[ "$sz" = "$(wc -c <.gitignore)" ] && ok "blanket .claude/ -> gitignore untouched" || no "appended despite blanket ignore"
+
+echo "-- new-lane: --sibling places outside the repo --"
+mkrepo "$SB/S"; cd "$SB/S"
+OS="$(bash "$NL" --sibling big 2>/dev/null)"
+[ "$OS" = "$SB/S-big" ] && ok "sibling path = $OS" || no "sibling=[$OS]"
+[ -d "$SB/S-big" ] && ok "sibling dir outside repo" || no "sibling dir missing"
+
+echo "-- new-lane: anchors at MAIN when invoked from inside a lane (no nesting) --"
+mkrepo "$SB/N"; cd "$SB/N"; L1="$(bash "$NL" first 2>/dev/null)"
+( cd "$L1" && bash "$NL" second >/dev/null 2>&1 )
+[ -d "$SB/N/.claude/worktrees/second" ] && ok "lane anchored to MAIN" || no "did not anchor to MAIN"
+[ ! -d "$L1/.claude/worktrees/second" ] && ok "no nested worktree inside the lane" || no "nested worktree leaked"
+
+echo "-- new-lane: validate-before-mutate + arg hygiene --"
+mkrepo "$SB/V"; cd "$SB/V"
+bash "$NL" lane1 no-such-branch >/dev/null 2>/dev/null; ee=$?
+[ $ee -eq 2 ] && ok "invalid base -> exit 2" || no "invalid base exit $ee"
+[ ! -f .gitignore ] && ok "invalid base left no .gitignore" || no "partial gitignore on bad base"
+bash "$NL" --frobnicate slug >/dev/null 2>/dev/null; ee=$?
+[ $ee -eq 2 ] && ok "unknown flag -> exit 2" || no "unknown flag exit $ee"
+[ -z "$(git branch --list 'lane/*')" ] && ok "unknown flag created no branch" || no "stray branch from bad flag"
+mkrepo "$SB/V2"; cd "$SB/V2"; git branch lane/dup
+bash "$NL" dup >/dev/null 2>/dev/null; ee=$?
+[ $ee -eq 1 ] && ok "existing branch -> exit 1" || no "dup exit $ee"
+[ ! -f .gitignore ] && ok "conflict left no .gitignore" || no "partial gitignore on conflict"
+bash "$NL" >/dev/null 2>/dev/null; [ $? -eq 2 ] && ok "no slug -> exit 2" || no "empty not exit 2"
+
+echo "-- new-lane: slug sanitised; syntax clean --"
+mkrepo "$SB/Z"; cd "$SB/Z"
+bash "$NL" "Feat Foo/Bar" >/dev/null 2>&1
+git show-ref --verify --quiet refs/heads/lane/feat-foobar && ok "slug 'Feat Foo/Bar' -> lane/feat-foobar" || no "slug sanitise wrong: $(git branch --list 'lane/*')"
+bash -n "$NL" && ok "new-lane.sh syntax OK" || no "new-lane.sh syntax error"
+
+if [ -n "$GUARD" ]; then
+  echo "-- worktree-guard: git clean force-level detection --"
+  bash -n "$GUARD" && ok "worktree-guard.sh syntax OK" || no "guard syntax error"
+  WG="$SB/WG"; mkrepo "$WG"; mkdir -p "$WG/.claude/worktrees"
+  gclean(){ local o; o=$(printf '{"tool_input":{"command":"%s"},"cwd":"%s"}' "$1" "$WG" | bash "$GUARD" 2>&1)
+    if [ "$2" = warn ]; then case "$o" in *double-force*) ok "guard WARN: $1";; *) no "guard expected WARN: $1 [${o:-silent}]";; esac
+    else [ -z "$o" ] && ok "guard silent: $1" || no "guard expected silent: $1"; fi; }
+  gclean "git clean -fdx"            silent
+  gclean "git clean -ffdx"          warn
+  gclean "git clean --force --force" warn
+  gclean "git clean -f -f"          warn
+  gclean "git clean -fd -fx"        warn
+  gclean "git clean --force -f"     warn
+  gclean "git clean -n"             silent
+  gclean "git clean -fd ./somepath" silent
+  # exemption: a session whose cwd is inside a worktree is not warned
+  ex=$(printf '{"tool_input":{"command":"git clean -ffdx"},"cwd":"%s/.claude/worktrees/foo"}' "$WG" | bash "$GUARD" 2>&1)
+  [ -z "$ex" ] && ok "guard exempt inside own worktree" || no "guard warned inside own worktree"
+  # existing rules unregressed
+  for c in "git add -A" "rm -rf .claude/worktrees/foo" "git worktree remove .claude/worktrees/foo"; do
+    o=$(printf '{"tool_input":{"command":"%s"},"cwd":"%s"}' "$c" "$WG" | bash "$GUARD" 2>&1)
+    [ -n "$o" ] && ok "guard still warns: $c" || no "guard regressed: $c"
+  done
+else
+  echo "  SKIP  worktree-guard.sh not found (hooks/ unavailable in this layout)"
+fi
+
+echo "=== $P passed, $F failed ==="
+[ $F -eq 0 ]