Close the last three: doctor + readiness coverage, one shared test helper

.github/workflows/test.yml

@@ -37,6 +37,10 @@ jobs:
         run: ./test/test-version-classifier.sh
       - name: Plugin-status unit tests
         run: ./test/test-plugin-status.sh
+      - name: Doctor behavioral tests
+        run: ./test/test-doctor.sh
+      - name: Status / readiness behavioral tests
+        run: ./test/test-status.sh
       - name: Preamble embedded-python tests
         run: ./test/test-preamble-python.sh
       - name: Setup behavioral tests

test/integration-test.sh

@@ -2,9 +2,7 @@
 # idstack integration tests — behavioral tests for bin scripts
 set -e
 
-PASS=0
-FAIL=0
-TOTAL=0
+. "$(dirname "$0")/test-helper.sh"
 
 IDSTACK_DIR="$(cd "$(dirname "$0")/.." && pwd)"
 TEST_DIR=$(mktemp -d)
@@ -16,21 +14,6 @@ trap 'rm -rf "$TEST_DIR"' EXIT
 # uncommitted work doesn't trip the check.
 TREE_BEFORE=$(git -C "$IDSTACK_DIR" status --porcelain 2>/dev/null || true)
 
-check() {
-  TOTAL=$((TOTAL + 1))
-  local _out
-  if _out=$(eval "$2" 2>&1); then
-    echo "  PASS: $1"
-    PASS=$((PASS + 1))
-  else
-    echo "  FAIL: $1"
-    if [ -n "$_out" ]; then
-      printf '%s\n' "$_out" | head -5 | sed 's/^/        | /'
-    fi
-    FAIL=$((FAIL + 1))
-  fi
-}
-
 echo "idstack integration tests"
 echo "  test dir: $TEST_DIR"
 echo ""

test/mutation-test.sh

@@ -149,6 +149,49 @@ open(p,'w').write(s)
 PY
 expect_fail "plugin-status fixed-window regression" "$WORK/r/test/test-plugin-status.sh"
 
+# 7b. doctor stops flagging a disabled install -> test-doctor must fail.
+# doctor exists to explain why /idstack:<skill> is missing; a branch that stops
+# reporting turns it into a script that always says everything is fine.
+fresh
+sed -i.bak 's|echo "  PROBLEM: idstack@idstack is installed but not enabled."|echo "  OK: installed"|' \
+  "$WORK/r/bin/idstack-doctor"
+expect_fail "doctor silently passes a disabled install" "$WORK/r/test/test-doctor.sh"
+
+# 7c. readiness threshold drifts by one -> test-status must fail.
+# A fixture failing every threshold at once cannot catch this: some other
+# unmet condition keeps the verdict NOT-READY either way. The boundary cases
+# in test-status.sh hold the other two dimensions passing so the constant is
+# the only thing that can flip the verdict.
+fresh
+sed -i.bak 's|^READY_QUALITY_MIN=70|READY_QUALITY_MIN=69|' "$WORK/r/bin/idstack-status"
+expect_fail "readiness quality threshold off by one" "$WORK/r/test/test-status.sh"
+
+# 7d. WCAG Level-A override removed from the verdict -> test-status must fail.
+# The override exists in access_tier() and again in verdict(); deleting either
+# alone leaves the other reporting, so both are pinned.
+fresh
+python3 - "$WORK/r/bin/idstack-status" <<'PY'
+import sys
+p = sys.argv[1]; s = open(p).read()
+s = s.replace("    if level_a_violations:\n        issues.append(f'{len(level_a_violations)} WCAG Level-A violation(s)')\n", "")
+open(p, 'w').write(s)
+PY
+expect_fail "WCAG Level-A override dropped from verdict" "$WORK/r/test/test-status.sh"
+
+# 7e. a suite regrows its own counters -> smoke-test must fail.
+# This is how the nine copies of check() drifted apart in the first place: each
+# suite kept a private PASS=0 and its own body, and one of them ended up
+# swallowing failure output entirely.
+fresh
+python3 - "$WORK/r/test/test-doctor.sh" <<'PY'
+import sys
+p = sys.argv[1]; s = open(p).read()
+s = s.replace('. "$(dirname "$0")/test-helper.sh"',
+              'PASS=0\nFAIL=0\nTOTAL=0\ncheck() { TOTAL=$((TOTAL+1)); eval "$2" >/dev/null 2>&1 && PASS=$((PASS+1)) || FAIL=$((FAIL+1)); }', 1)
+open(p, 'w').write(s)
+PY
+expect_fail "suite regrows private test counters" "$WORK/r/test/smoke-test.sh" "$WORK/r"
+
 # 8. version disagreement -> smoke-test must fail
 fresh
 printf '9.9.9.9\n' > "$WORK/r/VERSION"

test/smoke-test.sh

@@ -2,9 +2,7 @@
 # idstack smoke test — verifies installation is correct
 set -e
 
-PASS=0
-FAIL=0
-TOTAL=0
+. "$(dirname "$0")/test-helper.sh"
 
 # Verify the repo this script lives in (test/smoke-test.sh -> repo root is "..").
 # Override with $1 to point at a different checkout (CI fixtures, etc.).
@@ -15,21 +13,6 @@ IDSTACK_DIR="${1:-$(cd "$(dirname "$0")/.." && pwd -P)}"
 VER="$(tr -d '[:space:]' < "$IDSTACK_DIR/VERSION" 2>/dev/null || true)"
 VER3="${VER%.*}"  # 4-component 3.2.0.0 -> 3-component 3.2.0 (JSON-LD softwareVersion)
 
-check() {
-  TOTAL=$((TOTAL + 1))
-  local _out
-  if _out=$(eval "$2" 2>&1); then
-    echo "  PASS: $1"
-    PASS=$((PASS + 1))
-  else
-    echo "  FAIL: $1"
-    if [ -n "$_out" ]; then
-      printf '%s\n' "$_out" | head -5 | sed 's/^/        | /'
-    fi
-    FAIL=$((FAIL + 1))
-  fi
-}
-
 echo "idstack smoke test"
 echo "  idstack dir: $IDSTACK_DIR"
 echo ""
@@ -73,10 +56,23 @@ for script in idstack-migrate idstack-timeline-log idstack-learnings-log idstack
 done
 
 # Bash syntax gate for the shell entry points (idstack-manifest-merge is python).
-for script in setup bin/idstack-doctor bin/idstack-gen-skills bin/idstack-status bin/idstack-migrate bin/idstack-slugify bin/idstack-update-check bin/lib/version-classify.sh bin/lib/plugin-status.sh; do
+for script in setup bin/idstack-doctor bin/idstack-gen-skills bin/idstack-status bin/idstack-migrate bin/idstack-slugify bin/idstack-update-check bin/lib/version-classify.sh bin/lib/plugin-status.sh test/test-helper.sh; do
   check "$script passes bash -n" "bash -n '$IDSTACK_DIR/$script'"
 done
 
+# Every suite takes its counters from test/test-helper.sh. Nine suites had
+# each grown their own copy, and they had already drifted — two spelled the
+# helper `assert`, and one swallowed failure output entirely, so CI reported a
+# bare FAIL with no diagnostics. A local `PASS=0` is how that regrows.
+check "test-helper.sh exists" "[ -f '$IDSTACK_DIR/test/test-helper.sh' ]"
+for suite in smoke-test integration-test test-manifest-merge test-version-classifier \
+             test-plugin-status test-preamble-python test-setup test-doctor test-status; do
+  check "test/$suite.sh sources the shared helper" \
+    "grep -q 'test-helper.sh' '$IDSTACK_DIR/test/$suite.sh'"
+  check "test/$suite.sh defines no local counters" \
+    "! grep -qE '^(PASS|FAIL|TOTAL)=0' '$IDSTACK_DIR/test/$suite.sh'"
+done
+
 # Check template system
 check "templates/preamble.md exists" "[ -f '$IDSTACK_DIR/templates/preamble.md' ]"
 check "templates/report-format.md exists" "[ -f '$IDSTACK_DIR/templates/report-format.md' ]"
@@ -274,6 +270,15 @@ check "idstack-doctor no longer uses a fixed -A4 window" "! grep -q 'grep -A4' '
 if [ -x "$IDSTACK_DIR/test/test-plugin-status.sh" ]; then
   check "plugin-status unit tests pass" "'$IDSTACK_DIR/test/test-plugin-status.sh'"
 fi
+# Hooked in here as well as in CI because release.yml's gate runs only
+# smoke-test.sh — doctor's branches would otherwise be unverified at release.
+if [ -x "$IDSTACK_DIR/test/test-doctor.sh" ]; then
+  check "doctor behavioral tests pass" "'$IDSTACK_DIR/test/test-doctor.sh'"
+fi
+# --readiness is the pre-export gate; same reasoning as doctor above.
+if [ -x "$IDSTACK_DIR/test/test-status.sh" ]; then
+  check "idstack-status behavioral tests pass" "'$IDSTACK_DIR/test/test-status.sh'"
+fi
 
 # ./setup is what a new user runs first; it is exercised against a repo copy
 # with a fake $HOME and a stub `claude`, never the real install.

test/test-doctor.sh

@@ -0,0 +1,171 @@
+#!/usr/bin/env bash
+# Unit tests for bin/idstack-doctor.
+#
+# doctor's whole job is to tell a user why /idstack:<skill> isn't working, so
+# every branch it can print is a branch someone will act on. Before this suite
+# it had no execution coverage at all — smoke-test only checked the file exists
+# and parses.
+#
+# Two things this harness has to get right, both of which are easy to get
+# wrong:
+#
+#   1. doctor sources bin/lib/version-classify.sh and bin/lib/plugin-status.sh
+#      (see bin/idstack-doctor:21-23), so the fixture copies the whole bin/
+#      tree. Copying just the one script gives a "No such file" on the source
+#      line and every assertion below fails for the wrong reason.
+#
+#   2. PATH is replaced, not prepended. The "claude not found" case is only
+#      meaningful if `claude` is genuinely unreachable — with the real PATH
+#      still appended, that test passes on CI (no claude) and fails on any
+#      developer machine that has Claude Code installed.
+
+set -e
+
+. "$(dirname "$0")/test-helper.sh"
+
+REPO_DIR="$(cd "$(dirname "$0")/.." && pwd -P)"
+TEST_ROOT="$(mktemp -d)"
+trap 'rm -rf "$TEST_ROOT"' EXIT
+
+export HOME="$TEST_ROOT/home"
+mkdir -p "$HOME"
+
+# Fixture repo — whole bin/ tree, so the sourced libs resolve.
+MOCK_IDSTACK_DIR="$TEST_ROOT/idstack"
+mkdir -p "$MOCK_IDSTACK_DIR/bin"
+cp -R "$REPO_DIR/bin/." "$MOCK_IDSTACK_DIR/bin/"
+DOCTOR_CMD="$MOCK_IDSTACK_DIR/bin/idstack-doctor"
+
+# Pinned PATH: only what doctor and its libs actually invoke, plus whatever
+# mock we drop in. `claude` is absent unless a test installs it.
+MOCK_BIN="$TEST_ROOT/bin"
+mkdir -p "$MOCK_BIN"
+for _c in bash sh env command dirname echo grep head python3 readlink sed tr awk cat ls rm mkdir ln chmod mktemp printf sort wc basename find touch cp; do
+  _p=$(command -v "$_c" 2>/dev/null) && ln -sf "$_p" "$MOCK_BIN/$_c"
+done
+export PATH="$MOCK_BIN"
+
+SKILLS="needs-analysis learning-objectives course-quality-review course-import \
+assessment-design course-builder course-export accessibility-review red-team \
+pipeline learn"
+
+setup_happy_path() {
+  rm -rf "$MOCK_IDSTACK_DIR/.claude-plugin" "$MOCK_IDSTACK_DIR/skills" "$HOME/.claude"
+  mkdir -p "$MOCK_IDSTACK_DIR/.claude-plugin"
+  # Version must parse as JSON — doctor reads it with python3, not a regex.
+  echo '{"name": "idstack", "version": "9.9.9.9"}' > "$MOCK_IDSTACK_DIR/.claude-plugin/plugin.json"
+  echo '{"name": "idstack"}' > "$MOCK_IDSTACK_DIR/.claude-plugin/marketplace.json"
+  for skill in $SKILLS; do
+    mkdir -p "$MOCK_IDSTACK_DIR/skills/$skill"
+    touch "$MOCK_IDSTACK_DIR/skills/$skill/SKILL.md"
+  done
+}
+
+# Stub `claude` whose `plugin list` prints $1 verbatim.
+mock_claude_output() {
+  {
+    echo '#!/bin/sh'
+    echo 'if [ "$1 $2" = "plugin list" ]; then'
+    echo "cat << 'INNER_EOF'"
+    echo "$1"
+    echo 'INNER_EOF'
+    echo 'fi'
+  } > "$MOCK_BIN/claude"
+  chmod +x "$MOCK_BIN/claude"
+}
+
+remove_mock_claude() { rm -f "$MOCK_BIN/claude"; }
+
+echo "idstack doctor tests"
+echo ""
+
+# --- healthy install ---
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+check "healthy install reports OK and exits 0" "'$DOCTOR_CMD'" 0 "OK"
+
+# --- manifest problems ---
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+rm "$MOCK_IDSTACK_DIR/.claude-plugin/plugin.json"
+check "missing plugin.json is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: missing.*plugin\.json"
+
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+rm "$MOCK_IDSTACK_DIR/.claude-plugin/marketplace.json"
+check "missing marketplace.json is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: missing.*marketplace\.json"
+
+# plugin.json is parsed as JSON, not grepped — malformed must be caught.
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+echo 'not json at all' > "$MOCK_IDSTACK_DIR/.claude-plugin/plugin.json"
+check "unparseable plugin.json is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: could not parse version"
+
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+rm "$MOCK_IDSTACK_DIR/skills/needs-analysis/SKILL.md"
+check "missing SKILL.md is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: 1 SKILL\.md file"
+
+# --- claude install state ---
+setup_happy_path
+remove_mock_claude
+check "absent claude is a WARNING, not a failure" "'$DOCTOR_CMD'" 0 "WARNING: 'claude' not found on PATH"
+
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: disabled"
+check "installed but disabled is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: idstack@idstack is installed but not enabled"
+
+setup_happy_path
+mock_claude_output "some-other-plugin
+status: enabled"
+check "not installed is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: idstack@idstack is not installed"
+
+# The v3.3.0.0 regression: a neighbouring plugin's "enabled" line was read as
+# idstack's, so a disabled idstack was diagnosed healthy. Pinned here too, at
+# the doctor level rather than only in test-plugin-status.sh.
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: disabled
+other-plugin@other
+status: enabled"
+check "neighbour's enabled line is not attributed to idstack" "'$DOCTOR_CMD'" 1 "PROBLEM: idstack@idstack is installed but not enabled"
+
+# --- legacy install conflicts ---
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+mkdir -p "$HOME/.claude/skills"
+ln -s "$TEST_ROOT/dummy" "$HOME/.claude/skills/idstack"
+check "legacy dispatcher symlink is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: legacy symlink at"
+
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+mkdir -p "$HOME/.claude/skills/idstack"
+echo "2.0.0" > "$HOME/.claude/skills/idstack/VERSION"
+check "pre-v2.0.1.0 install dir is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: pre-v2\.0\.1\.0 install at"
+
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+mkdir -p "$HOME/.claude/skills"
+ln -s "$TEST_ROOT/dummy-idstack/some-skill" "$HOME/.claude/skills/needs-analysis"
+check "pre-v2 per-skill symlink is a PROBLEM" "'$DOCTOR_CMD'" 1 "PROBLEM: pre-v2 skill symlink"
+
+# An unrelated directory must not be claimed as a broken idstack install.
+setup_happy_path
+mock_claude_output "idstack@idstack
+status: enabled"
+mkdir -p "$HOME/.claude/skills/idstack"
+echo "someone else's thing" > "$HOME/.claude/skills/idstack/README.md"
+check "unrecognized dir warns rather than claiming ownership" "'$DOCTOR_CMD'" 0 "WARNING: directory at"
+
+echo ""
+echo "  $PASS/$TOTAL passed"
+[ "$FAIL" = "0" ] || exit 1

test/test-helper.sh

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+# Shared counters and assertion helper for the idstack test suites.
+# Sourced, never executed: `. "$(dirname "$0")/test-helper.sh"`.
+#
+# Nine suites each carried their own PASS/FAIL/TOTAL and their own copy of this
+# function. They had already drifted: some printed failure diagnostics, some
+# swallowed them with 2>/dev/null, and two spelled the helper `assert` instead
+# of `check`. A suite that prints nothing on failure is the worst version —
+# CI tells you something broke and not what.
+#
+# Suites with a genuinely different assertion shape keep their own wrapper and
+# use only the counters from here: test-version-classifier.sh compares a
+# version string to an expected classification rather than running a command.
+
+PASS=0
+FAIL=0
+TOTAL=0
+
+# check <description> <command> [expected_exit] [expected_output_regex]
+#
+# Runs <command> through eval with stderr folded into stdout. With no optional
+# arguments this is the plain "command must succeed" form every suite already
+# used. Pass expected_exit to assert a specific non-zero code (exit codes are
+# part of the contract for bin/idstack-manifest-merge and bin/idstack-doctor),
+# and expected_output_regex to also require matching output — an ERE, matched
+# with grep -qE.
+#
+# On failure the first 5 lines of output are printed, indented. Never silence
+# this; a bare FAIL is the thing this helper exists to stop.
+check() {
+  TOTAL=$((TOTAL + 1))
+  local _desc="$1" _cmd="$2" _want_exit="${3:-0}" _want_out="${4:-}"
+  local _out _code=0
+
+  _out=$(eval "$_cmd" 2>&1) || _code=$?
+
+  if [ "$_code" -ne "$_want_exit" ]; then
+    FAIL=$((FAIL + 1))
+    echo "  FAIL: $_desc (expected exit $_want_exit, got $_code)"
+    [ -n "$_out" ] && printf '%s\n' "$_out" | head -5 | sed 's/^/        | /'
+    return 0
+  fi
+
+  if [ -n "$_want_out" ] && ! printf '%s\n' "$_out" | grep -qE "$_want_out"; then
+    FAIL=$((FAIL + 1))
+    echo "  FAIL: $_desc (exit $_code correct, output missing /$_want_out/)"
+    [ -n "$_out" ] && printf '%s\n' "$_out" | head -5 | sed 's/^/        | /'
+    return 0
+  fi
+
+  PASS=$((PASS + 1))
+  echo "  PASS: $_desc"
+  return 0
+}