From b2aa81f93ccf1dbfc1fb8e4d75b524c41e87b07f Mon Sep 17 00:00:00 2001 From: Logan Gagne Date: Sat, 29 Aug 2026 18:06:19 -0400 Subject: [PATCH] chore: single-source the CI checks and share one guarded git mock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two near-misses from the last session had the same shape: something was maintained in two places, the copies drifted, and the drift failed silently. **The check list.** CLAUDE.md said "CI runs four independent checks" and listed four; CI runs five. The missing one, shell linting, lived inline in the workflow YAML, so there was no way to reproduce it locally at all — you could pass every documented check and still fail CI. The markdown entry was also `rumdl .`, which is not a valid invocation and linted nothing. - Extract the inline shellcheck job into .github/scripts/lint-shell.sh and have the workflow call it, so CI is reproducible locally - Add .github/scripts/validate-all.sh running all five with a pass/fail summary, and point CLAUDE.md at that single command instead of a list that can rot independently - lint-shell.sh treats a missing shellcheck as a warning locally but a hard failure when CI is set, so a broken runner image cannot go silently green **The git mock.** It was hand-copied into nine suites, and every copy delegated with `command git "$@"`. `command` bypasses functions and aliases but not PATH lookup, and the mock's directory is prepended to PATH, so each copy re-executed itself without bound. - Add tests/lib/mock-git.sh with a single write_mock_git, guarded twice: delegation strips the mock's own directory from PATH, and a depth counter aborts on the third re-entry so a future delegation mistake fails loudly instead of forking forever - Convert all nine suites to it, removing 119 lines of duplication - lint-shell.sh fails the build if `command git|gh|tea` reappears in tests/ Verified rather than assumed: the depth guard fires (exit 1, "recursion detected ... depth 3") against a deliberately broken delegation; the lint backstop both passes clean and rejects a reintroduced mock; validate-all.sh propagates failure (exit 1) rather than always succeeding; suite discovery still finds 32 suites and ignores the new lib. validate-all.sh: all five checks pass, 1857 tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EddJxaTMXyj7o4ndrqfCxa --- .github/scripts/lint-shell.sh | 55 ++++++++++++++++++++ .github/scripts/validate-all.sh | 58 ++++++++++++++++++++++ .github/workflows/ci.yml | 20 +------- CLAUDE.md | 34 ++++++++++++- tests/git-cli/test-body-args.sh | 12 ++--- tests/git-cli/test-issue-write-json.sh | 19 ++----- tests/git-cli/test-pr-create.sh | 12 ++--- tests/git-cli/test-pr-show-gitea.sh | 10 +--- tests/git-cli/test-run-show.sh | 10 +--- tests/git-cli/test-run-watch-gitea.sh | 12 ++--- tests/lib/mock-git.sh | 55 ++++++++++++++++++++ tests/session/test-ci-poll.sh | 28 ++--------- tests/session/test-pr-auto-merge-status.sh | 10 +--- tests/session/test-pr-wait.sh | 10 +--- 14 files changed, 226 insertions(+), 119 deletions(-) create mode 100755 .github/scripts/lint-shell.sh create mode 100755 .github/scripts/validate-all.sh create mode 100644 tests/lib/mock-git.sh diff --git a/.github/scripts/lint-shell.sh b/.github/scripts/lint-shell.sh new file mode 100755 index 0000000..49d353d --- /dev/null +++ b/.github/scripts/lint-shell.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# lint-shell.sh — shellcheck every shell script in the repo. +# +# Extracted from .github/workflows/ci.yml so the check is reproducible +# locally. CI calls this script; do not re-inline the logic into the workflow. +# +# Usage: bash .github/scripts/lint-shell.sh + +set -euo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/../.." + +if ! command -v shellcheck >/dev/null 2>&1; then + # Missing shellcheck is a hard failure in CI — skipping there would turn a + # broken runner image into a silently green build. Locally it is a warning, + # so the script stays usable without forcing an install. + if [[ -n "${CI:-}" ]]; then + echo "shellcheck not installed and CI is set — refusing to skip" >&2 + exit 1 + fi + echo "shellcheck not installed — skipping (CI still enforces it)" >&2 + echo " install: sudo pacman -S shellcheck | brew install shellcheck" >&2 + exit 0 +fi + +# Collect .sh files (exclude symlinks to avoid double-checking) +mapfile -t scripts < <(find . -name '*.sh' -not -path './.git/*' -not -type l | sort) + +# Collect extensionless scripts in scripts/ dirs (exclude symlinks) +while IFS= read -r f; do + [[ -L "$f" ]] && continue + if head -1 "$f" 2>/dev/null | grep -qE '^#!/.*(bash|sh)'; then + scripts+=("$f") + fi +done < <(find . -path '*/scripts/*' -not -name '*.*' -not -path './.git/*' -type f | sort) + +if [[ ${#scripts[@]} -eq 0 ]]; then + echo "No shell scripts found" + exit 0 +fi + +echo "Checking ${#scripts[@]} script(s)..." +shellcheck --severity=error "${scripts[@]}" + +# Backstop: a PATH-injected mock must never delegate with `command `. +# `command` bypasses functions and aliases but NOT PATH lookup, so a mock whose +# directory is first on PATH re-executes itself without bound. Use +# tests/lib/mock-git.sh, which strips its own directory from PATH and carries a +# recursion depth guard. +if grep -rn --include='*.sh' -E '\)\s*command (git|gh|tea) ' tests/ 2>/dev/null; then + echo "" >&2 + echo "ERROR: mock delegates via 'command ' — this recurses through PATH." >&2 + echo "Use write_mock_git from tests/lib/mock-git.sh instead." >&2 + exit 1 +fi diff --git a/.github/scripts/validate-all.sh b/.github/scripts/validate-all.sh new file mode 100755 index 0000000..fd1be49 --- /dev/null +++ b/.github/scripts/validate-all.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# validate-all.sh — run every check CI runs, in one command. +# +# This is the single source of truth for "what CI will check". CLAUDE.md points +# here rather than listing the commands, so the docs cannot drift out of sync +# with the workflow the way `rumdl .` did (it was never a valid invocation, so +# the documented markdown lint silently never ran). +# +# Each check is also a CI job in .github/workflows/ci.yml, which runs them in +# parallel for per-job status. Keep the two lists in step: if you add a job +# there, add it here. +# +# Usage: bash .github/scripts/validate-all.sh + +set -uo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/../.." + +declare -a names=() results=() +failed=0 + +run_check() { + local name="$1" + shift + echo "===============================" + echo "=== $name" + echo "===============================" + names+=("$name") + if "$@"; then + results+=("PASS") + else + results+=("FAIL") + failed=1 + fi + echo "" +} + +run_check "plugin tests" bash test.sh +run_check "plugin structure" bash .github/scripts/validate-plugins.sh +run_check "frontmatter" bash .github/scripts/validate-frontmatter.sh +run_check "markdown lint" rumdl check . +run_check "shell lint" bash .github/scripts/lint-shell.sh + +echo "===============================" +echo "=== Summary" +echo "===============================" +for i in "${!names[@]}"; do + printf " %-20s [%s]\n" "${names[$i]}" "${results[$i]}" +done +echo "" + +if [[ $failed -eq 0 ]]; then + echo "All checks passed." +else + echo "Some checks failed." +fi + +exit "$failed" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42aa023..44b5c24 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,22 +56,4 @@ jobs: steps: - uses: actions/checkout@v5 - name: Find and lint shell scripts - run: | - # Collect .sh files (exclude symlinks to avoid double-checking) - mapfile -t scripts < <(find . -name '*.sh' -not -path './.git/*' -not -type l | sort) - - # Collect extensionless scripts in scripts/ dirs (exclude symlinks) - while IFS= read -r f; do - [[ -L "$f" ]] && continue - if head -1 "$f" 2>/dev/null | grep -qE '^#!/.*(bash|sh)'; then - scripts+=("$f") - fi - done < <(find . -path '*/scripts/*' -not -name '*.*' -not -path './.git/*' -type f | sort) - - if [[ ${#scripts[@]} -eq 0 ]]; then - echo "No shell scripts found" - exit 0 - fi - - echo "Checking ${#scripts[@]} script(s)..." - shellcheck --severity=error "${scripts[@]}" + run: bash .github/scripts/lint-shell.sh diff --git a/CLAUDE.md b/CLAUDE.md index 7043fad..bdffb17 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -210,21 +210,53 @@ This is a GitHub-hosted repository. Use `gh` for all GitHub operations (PRs, iss ### Validation -CI runs four independent checks. Run all locally before pushing: +CI runs five independent checks. Run all of them locally with one command: + +```bash +bash .github/scripts/validate-all.sh +``` + +**Prefer that over running the checks by hand.** It is the single source of +truth for what CI enforces, so the list cannot rot the way a copy in this file +does — it previously documented `rumdl .`, which is not a valid invocation and +silently linted nothing, and it omitted shell linting entirely. + +The individual checks, if you need to run one in isolation: ```bash bash test.sh # plugin tests bash .github/scripts/validate-plugins.sh # plugin structure bash .github/scripts/validate-frontmatter.sh # command/skill frontmatter rumdl check . # markdown linting +bash .github/scripts/lint-shell.sh # shellcheck + mock-recursion guard ``` +Adding a CI job means adding it to `validate-all.sh` too; the script says so. + Run a single test suite directly: ```bash bash tests/permission-manager/test-*.sh ``` +### Test mocks + +Suites that need a fake `git` on `PATH` must use `write_mock_git` from +`tests/lib/mock-git.sh` rather than hand-writing one: + +```bash +source "$SCRIPT_DIR/../lib/mock-git.sh" +write_mock_git "$MOCK_DIR" "https://github.com/owner/repo.git" +``` + +Never delegate unmatched subcommands with `command git "$@"`. `command` bypasses +functions and aliases but **not** `PATH` lookup, so a mock whose directory is +first on `PATH` re-executes itself without bound — this went unnoticed across +nine suites until something called a second git subcommand, then spawned 56k +processes and exhausted the terminal's cgroup pid limit. The shared helper +strips its own directory from `PATH` before delegating and carries a recursion +depth guard; `lint-shell.sh` fails the build if the old form reappears. + ### Git hooks Opt into the repo's pre-commit hooks (checks vendored utils drift) once per clone: diff --git a/tests/git-cli/test-body-args.sh b/tests/git-cli/test-body-args.sh index 66e3784..4252769 100644 --- a/tests/git-cli/test-body-args.sh +++ b/tests/git-cli/test-body-args.sh @@ -10,6 +10,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -43,15 +44,8 @@ skip_filtered() { } # Mock git so platform detection resolves to github. -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://github.com/owner/repo.git" ;; - "config user.name") echo "testuser" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://github.com/owner/repo.git" \ + ' "config user.name") echo "testuser" ;;' # Mock gh: capture the inline body into BODY_FILE. issue/pr create still pass it # via --body; issue/pr comment now post via `gh api ... -f body=VALUE`, so capture diff --git a/tests/git-cli/test-issue-write-json.sh b/tests/git-cli/test-issue-write-json.sh index c81cef9..0655029 100644 --- a/tests/git-cli/test-issue-write-json.sh +++ b/tests/git-cli/test-issue-write-json.sh @@ -17,6 +17,7 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -103,14 +104,7 @@ EOF # Platform selectors: choose the remote host so detect_platform resolves # github (github.com fallback) vs gitea (matches the tea login host). set_platform_github() { - cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://github.com/owner/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF - chmod +x "$MOCK_DIR/git" + write_mock_git "$MOCK_DIR" "https://github.com/owner/repo.git" # tea present but with a non-github login → no match → github.com fallback. cat >"$MOCK_DIR/tea" <<'EOF' #!/usr/bin/env bash @@ -122,14 +116,7 @@ EOF } set_platform_gitea() { - cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF - chmod +x "$MOCK_DIR/git" + write_mock_git "$MOCK_DIR" "https://git.stonefish.tech/owner/repo.git" write_tea_mock } diff --git a/tests/git-cli/test-pr-create.sh b/tests/git-cli/test-pr-create.sh index 874f4f7..4c23a4b 100644 --- a/tests/git-cli/test-pr-create.sh +++ b/tests/git-cli/test-pr-create.sh @@ -8,6 +8,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -70,15 +71,8 @@ MOCK_HEADER } # Mock git so platform detection works (returns github.com remote) -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://github.com/owner/repo.git" ;; - "config user.name") echo "testuser" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://github.com/owner/repo.git" \ + ' "config user.name") echo "testuser" ;;' # --------------------------------------------------------------------------- # Test: --base omitted → auto-detects default branch diff --git a/tests/git-cli/test-pr-show-gitea.sh b/tests/git-cli/test-pr-show-gitea.sh index 8b03a71..c5a084a 100755 --- a/tests/git-cli/test-pr-show-gitea.sh +++ b/tests/git-cli/test-pr-show-gitea.sh @@ -14,6 +14,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -46,14 +47,7 @@ skip_filter() { # --------------------------------------------------------------------------- # Mock git: report a Gitea remote so platform detection resolves to "gitea". -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://git.stonefish.tech/owner/repo.git" # Mock tea: # - login list → advertise a login for the remote host (so platform == gitea) diff --git a/tests/git-cli/test-run-show.sh b/tests/git-cli/test-run-show.sh index 50921a6..5a7a94a 100755 --- a/tests/git-cli/test-run-show.sh +++ b/tests/git-cli/test-run-show.sh @@ -13,6 +13,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -45,14 +46,7 @@ skip_filter() { # --------------------------------------------------------------------------- # Mock git: report a Gitea remote so platform detection resolves to "gitea". -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://git.stonefish.tech/owner/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://git.stonefish.tech/owner/repo.git" # Mock tea: # - login list → advertise a login for the remote host (so platform == gitea) diff --git a/tests/git-cli/test-run-watch-gitea.sh b/tests/git-cli/test-run-watch-gitea.sh index 92d5c0d..0e12247 100755 --- a/tests/git-cli/test-run-watch-gitea.sh +++ b/tests/git-cli/test-run-watch-gitea.sh @@ -17,6 +17,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -54,15 +55,8 @@ skip_filter() { # Mock git: Gitea remote + a configurable rev-parse SHA for the branch. write_git_mock() { local sha="$1" - cat >"$MOCK_DIR/git" < [extra_case_arms] +write_mock_git() { + local dir="$1" remote="$2" extra="${3:-}" + + { + cat <<'MOCK_PRELUDE' +#!/usr/bin/env bash +# Generated by tests/lib/mock-git.sh — do not hand-copy this file. +if [ "${MOCK_GIT_DEPTH:-0}" -gt 2 ]; then + echo "mock git: recursion detected on '$*' (depth ${MOCK_GIT_DEPTH})" >&2 + exit 1 +fi +export MOCK_GIT_DEPTH=$((${MOCK_GIT_DEPTH:-0} + 1)) +case "$*" in +MOCK_PRELUDE + + printf ' "remote get-url origin") echo "%s" ;;\n' "$remote" + [[ -n "$extra" ]] && printf '%s\n' "$extra" + + cat <<'MOCK_CODA' + # Strip this mock's own directory from PATH before delegating, so `git` + # resolves to the real binary rather than back to this script. + *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; +esac +MOCK_CODA + } >"$dir/git" + + chmod +x "$dir/git" +} diff --git a/tests/session/test-ci-poll.sh b/tests/session/test-ci-poll.sh index b129e1d..64d8e42 100644 --- a/tests/session/test-ci-poll.sh +++ b/tests/session/test-ci-poll.sh @@ -8,6 +8,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -59,14 +60,7 @@ MOCK_HEADER } # Also mock git so platform detection works (returns github.com remote) -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://github.com/test/repo.git" # --------------------------------------------------------------------------- # Test: pass — run completes with success @@ -615,14 +609,7 @@ echo "── run watch: Gitea outcomes ──" # Switch the git mock to a Gitea-style remote so detect_platform takes the # gitea branch. Restored at the end of the section. -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://gitea.example.com/owner/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://gitea.example.com/owner/repo.git" # tea login list — the wrapper matches a configured login host against the # remote host to identify the platform. Return a single login matching the @@ -1042,14 +1029,7 @@ else fi # Restore the GitHub git mock so any future tests added below still see github. -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://github.com/test/repo.git" rm -f "$MOCK_DIR/tea" # --------------------------------------------------------------------------- diff --git a/tests/session/test-pr-auto-merge-status.sh b/tests/session/test-pr-auto-merge-status.sh index 3898228..8891880 100644 --- a/tests/session/test-pr-auto-merge-status.sh +++ b/tests/session/test-pr-auto-merge-status.sh @@ -8,6 +8,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -61,14 +62,7 @@ MOCK_HEADER } # Mock git so platform detection works (returns github.com remote) -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://github.com/test/repo.git" # --------------------------------------------------------------------------- # Test: Auto-merge enabled diff --git a/tests/session/test-pr-wait.sh b/tests/session/test-pr-wait.sh index b62912f..ae1edb8 100644 --- a/tests/session/test-pr-wait.sh +++ b/tests/session/test-pr-wait.sh @@ -8,6 +8,7 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" GIT_CLI="$SCRIPT_DIR/../../utils/git-cli" +source "$SCRIPT_DIR/../lib/mock-git.sh" PASS=0 FAIL=0 @@ -59,14 +60,7 @@ MOCK_HEADER } # Mock git so platform detection works (returns github.com remote) -cat >"$MOCK_DIR/git" <<'EOF' -#!/usr/bin/env bash -case "$*" in - "remote get-url origin") echo "https://github.com/test/repo.git" ;; - *) PATH=${PATH#"${0%/*}":}; exec git "$@" ;; -esac -EOF -chmod +x "$MOCK_DIR/git" +write_mock_git "$MOCK_DIR" "https://github.com/test/repo.git" # --------------------------------------------------------------------------- # Test: PR already merged (GitHub state=merged)