fix: address all 9 PR review feedback items

Agent-Logs-Url: https://github.com/GoCodeAlone/claude-superpowers/sessions/333f45fb-54f9-4471-a162-9d3bdd01cb0e

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

Author: Copilot

hooks/pre-compact-snapshot

@@ -26,17 +26,6 @@ hook_input=$(cat || true)
 cwd_dir=$(printf '%s' "$hook_input" | jq -r '.cwd // empty' 2>/dev/null || true)
 [ -z "$cwd_dir" ] && cwd_dir="${PWD}"
 
-# ── Portable sha256 (same helper as tests/plan-scope-check.sh) ───────────────
-sha256_file() {
-    if command -v sha256sum >/dev/null 2>&1; then
-        sha256sum "$1" 2>/dev/null | awk '{print $1}'
-    elif command -v shasum >/dev/null 2>&1; then
-        shasum -a 256 "$1" 2>/dev/null | awk '{print $1}'
-    else
-        echo "unavailable"
-    fi
-}
-
 # ── Find locked plans and collect their state ─────────────────────────────────
 plans_dir="${cwd_dir}/docs/plans"
 state_section=""
@@ -50,10 +39,12 @@ if [ -d "$plans_dir" ]; then
         # Extract Status line
         status_line=$(grep '\*\*Status:\*\*' "$plan" 2>/dev/null | tail -1 | sed 's/.*\*\*Status:\*\*[[:space:]]*//' || true)
 
-        # Hash of the lock file (the manifest hash stored at lock time)
+        # Read the manifest hash stored inside the lock file (first non-comment,
+        # non-blank line). This is the same value tests/plan-scope-check.sh
+        # --verify-lock compares against the current manifest sha256.
         if [ -f "$lock_file" ]; then
-            lock_hash=$(sha256_file "$lock_file")
-            state_section="${state_section}  ${plan_name}: ${status_line} (lock-file sha256: ${lock_hash})\n"
+            manifest_hash=$(awk 'NF && !/^#/ {print; exit}' "$lock_file" 2>/dev/null || true)
+            state_section="${state_section}  ${plan_name}: ${status_line} (manifest-sha256: ${manifest_hash:-unknown})\n"
         else
             state_section="${state_section}  ${plan_name}: ${status_line} (no lock file — not yet locked or lock file missing)\n"
         fi

hooks/pre-tool-scope-guard

@@ -68,7 +68,14 @@ case "$tool_name" in
     #   SUPERPOWERS_ALLOW_DEFAULT_BRANCH=1 git push origin main
     #   env SUPERPOWERS_SCOPE_LOCK_WRITE=1 bash -c '...'
     #   SUPERPOWERS_PLAN_LOCK_WRITE=1; git commit ...  (before semicolon)
-    if printf '%s' "$cmd" | grep -qE '(^|[;&|[:space:]])(export[[:space:]]+)?SUPERPOWERS_[A-Z_]+='; then
+    # Not blocked (benign debugging/grepping):
+    #   echo "SUPERPOWERS_HOOKS_DISABLE=1"
+    #   grep 'SUPERPOWERS_.*=' file
+    # Best-effort: strip single- and double-quoted strings before checking so
+    # that mentions of SUPERPOWERS_* inside quoted arguments don't trigger a
+    # false-positive block.
+    cmd_no_quotes=$(printf '%s' "$cmd" | sed "s/\"[^\"]*\"//g; s/'[^']*'//g")
+    if printf '%s' "$cmd_no_quotes" | grep -qE '(^|[;&|[:space:]])(export[[:space:]]+)?SUPERPOWERS_[A-Z_]+='; then
         block "Self-bypass attempt blocked: setting a SUPERPOWERS_* environment variable from inside a Bash tool call is not permitted. These variables control pipeline enforcement gates and may only be set by the human operator in their terminal environment before starting the agent session. The agent cannot grant itself permission to bypass its own constraints — that would make the gates meaningless."
     fi
 

hooks/subagent-scope-guard

@@ -43,8 +43,8 @@ violations=""
 
 # Working-tree and index modifications to protected files
 if command -v git >/dev/null 2>&1; then
-    (
-        cd "$cwd_dir" 2>/dev/null || exit 0
+    _saved_pwd="${PWD}"
+    if cd "$cwd_dir" 2>/dev/null; then
 
         # Uncommitted changes to scope-lock files
         scope_lock_dirty=$(git status --porcelain 2>/dev/null \
@@ -86,7 +86,9 @@ if command -v git >/dev/null 2>&1; then
                 done <<< "$plans_in_commit"
             fi
         fi
-    )
+
+        cd "$_saved_pwd"
+    fi
 fi
 
 [ -z "$violations" ] && exit 0

skills/scope-lock/SKILL.md

@@ -108,7 +108,7 @@ The unlock path is intentionally heavyweight. Cheap unlock = no lock at all.
 - Commit both files in the same commit: `chore: lock scope for <feature> (alignment passed)`.
 
 **`subagent-driven-development` (per-task checkpoint):**
-- Before dispatching the next task, run `tests/plan-scope-check.sh --plan <plan-path>` to verify (a) the plan's manifest hash still matches `<plan-path>.scope-lock`, (b) every commit on the feature branch traces to a task in the manifest, (c) no manifest task is missing.
+- Before dispatching the next task, run `tests/plan-scope-check.sh --verify-lock <plan-path>` to verify (a) the plan's manifest hash still matches `<plan-path>.scope-lock`, (b) every commit on the feature branch traces to a task in the manifest, (c) no manifest task is missing.
 - On any FAIL, stop dispatching new work; surface the discrepancy to the user.
 - After all tasks complete, run the same check before invoking `finishing-a-development-branch`.
 

tests/plan-scope-check.sh

@@ -13,9 +13,7 @@
 #                            <path>.scope-lock (only meaningful after the plan is
 #                            in Locked status).
 #   --against-branch <plan>  Verify the actual git branch layout matches the
-#                            PR Grouping table: every commit since the merge-base
-#                            with the plan's base branch is reachable from a
-#                            branch listed in the table; every branch in the
+#                            PR Grouping table: every branch listed in the
 #                            table exists locally or on origin.
 #
 # Multiple modes can be combined. With no flags, runs --plan on every plan in
@@ -66,11 +64,10 @@ sha256_stdin() {
 }
 
 # Check the manifest is well-formed. Args: plan path. Echoes problems to stdout.
-# Legacy plans (no manifest section AND no `# scope-manifest: required` marker
-# in a hidden HTML comment) are skipped — only plans that opt into the format
-# are enforced. New plans created by writing-plans always include the section,
-# so this only matters for grandfathering historical plans pre-dating the
-# scope-lock skill.
+# Legacy plans (no manifest section) are skipped — only plans that opt into the
+# format are enforced. New plans created by writing-plans always include the
+# section, so this only matters for grandfathering historical plans pre-dating
+# the scope-lock skill.
 check_manifest_wellformed() {
   local plan="$1"
   local manifest

tests/skill-activation-audit.sh

@@ -67,15 +67,24 @@ if [ ! -r "$STATE_FILE" ]; then
 fi
 
 # Pipeline gates we expect for an autonomous run, in order. The pipeline
-# is the canonical chain documented in skills/using-superpowers/SKILL.md.
+# is the canonical chain documented in skills/using-superpowers/SKILL.md:
+#   brainstorming → adversarial-design-review (design) → writing-plans →
+#   adversarial-design-review (plan) → alignment-check → scope-lock →
+#   subagent-driven-development → finishing-a-development-branch →
+#   pr-monitoring → post-merge-retrospective
+# Note: adversarial-design-review appears twice (design and plan phases);
+# this list de-dupes it — the audit reports the count seen so gaps can be
+# identified but cannot distinguish the two phases without --phase= args.
 PIPELINE_GATES=(
   brainstorming
   adversarial-design-review
   writing-plans
   alignment-check
+  scope-lock
   subagent-driven-development
   finishing-a-development-branch
   pr-monitoring
+  post-merge-retrospective
 )
 
 # Optional gates — present only when conditions trigger them. Reported
@@ -112,10 +121,10 @@ extract_skills() {
 
 extract_agents() {
   if command -v jq >/dev/null 2>&1; then
-    jq -r 'select(.tool=="Agent" or .tool=="Task") | .detail' "$STATE_FILE" 2>/dev/null \
+    jq -r 'select(.tool=="Agent" or (.tool | type=="string" and startswith("Task"))) | .detail' "$STATE_FILE" 2>/dev/null \
       | sed -nE 's/.*agent=([A-Za-z0-9_-]+).*/\1/p'
   else
-    grep -E '"tool":"(Agent|Task)"' "$STATE_FILE" 2>/dev/null \
+    grep -E '"tool":"(Agent|Task[^"]*)"' "$STATE_FILE" 2>/dev/null \
       | sed -nE 's/.*agent=([A-Za-z0-9_-]+).*/\1/p'
   fi
 }

tests/skill-cross-refs.sh

@@ -31,8 +31,8 @@ tmp_failures="$(mktemp)" || { echo "ERROR: mktemp failed" >&2; exit 3; }
 trap 'rm -f "$tmp_failures"' EXIT
 
 # Build the set of known skill names and agent names from the filesystem.
-known_skills="$(find skills -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | sort -u)"
-known_agents="$(find agents -mindepth 1 -maxdepth 1 -type f -name '*.md' -printf '%f\n' | sed -E 's|\.md$||' | sort -u)"
+known_skills="$(find skills -mindepth 1 -maxdepth 1 -type d | sed -E 's|.*/||' | sort -u)"
+known_agents="$(find agents -mindepth 1 -maxdepth 1 -type f -name '*.md' | sed -E 's|.*/||; s|\.md$||' | sort -u)"
 
 # Helper: is the name a known skill or agent?
 is_known_target() {
@@ -68,12 +68,14 @@ strip_fences() {
 
 # Files to scan. Exclude *creation-log* / changelog-style files where the
 # point is to record historical names that no longer exist.
-mapfile -t scan_files < <(find skills agents -type f -name '*.md' \
-  ! -iname 'CREATION-LOG.md' | sort)
+# Use a newline-separated string for portability (no mapfile / Bash 4+).
+scan_files_list="$(find skills agents -type f -name '*.md' \
+  ! -iname 'CREATION-LOG.md' | sort)"
 
 # --- 1. Skill / agent references ----------------------------------------
 
-for f in "${scan_files[@]}"; do
+while IFS= read -r f; do
+  [ -z "$f" ] && continue
   annotated="$(strip_fences "$f")"
 
   # Pattern 1: bare `<slug>/SKILL.md` references
@@ -118,7 +120,7 @@ for f in "${scan_files[@]}"; do
       fi
     done
   done < <(printf '%s\n' "$annotated" | grep -E 'superpowers:[a-z][a-z0-9-]+' || true)
-done
+done <<< "$scan_files_list"
 
 # --- 2. Step references --------------------------------------------------
 
@@ -141,7 +143,8 @@ has_step() {
     || grep -qE "Step[[:space:]]+${step}[[:space:]]*[:.]" "$file"
 }
 
-for f in "${scan_files[@]}"; do
+while IFS= read -r f; do
+  [ -z "$f" ] && continue
   annotated="$(strip_fences "$f")"
 
   while IFS=: read -r line_no line; do
@@ -162,7 +165,7 @@ for f in "${scan_files[@]}"; do
               | sed -E "s/^([a-z][a-z0-9-]+)[a-z']*[[:space:]]+Step[[:space:]]+([0-9]+[a-z]?).*/\1 \2/")
   done < <(printf '%s\n' "$annotated" \
             | grep -E "[a-z][a-z0-9-]+[a-z']*[[:space:]]+Step[[:space:]]+[0-9]+[a-z]?" || true)
-done
+done <<< "$scan_files_list"
 
 # --- Report --------------------------------------------------------------