From f0f612e4e51105af3feb869e9a50fc807dcc9cee Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:40:45 -0400 Subject: [PATCH 1/2] fix(repo): make check-affected list a branch's changes from any directory and fail on an empty list check-affected.sh took the tree to check from the working directory. Run by path from a worktree while the shell sat in the main checkout, it checked the main checkout, whose HEAD was at its merge-base with origin/main, found 0 changed files, skipped the tests as a foreign tree, and passed. That is the "7 passed, 7 skipped over 0 changed files" the ready gate recorded on #9041, #9048 and #9062, explicit --base included. The tree checked is now CHECK_DATA_ROOT's, else the one holding the script, as in the other check scripts. The pre-push hook names the branch tree for its merge-base copy, and the --uncommitted run names its snapshot. An empty changed set while HEAD is not the base exits 2 with a message instead of passing; --uncommitted with no edits checks HEAD itself so it still passes. Closes #9051 Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/check-affected-pre-push.sh | 2 +- scripts/check-affected.sh | 37 +++++++++----- scripts/check-affected.test.sh | 61 +++++++++++++++++++++--- 3 files changed, 80 insertions(+), 20 deletions(-) diff --git a/.claude/hooks/check-affected-pre-push.sh b/.claude/hooks/check-affected-pre-push.sh index 1684408402..f781af9245 100755 --- a/.claude/hooks/check-affected-pre-push.sh +++ b/.claude/hooks/check-affected-pre-push.sh @@ -287,7 +287,7 @@ run_checks() ( extract_tools "$top" "$mb" "$tmp/tools" || exit 2 fi [[ -x "$script" ]] || exit 2 - (cd "$data" && bash "$script" 2>&1) || rc=$? + (cd "$data" && CHECK_DATA_ROOT="$data" bash "$script" 2>&1) || rc=$? exit "$rc" ) diff --git a/scripts/check-affected.sh b/scripts/check-affected.sh index 17bb3f7221..8de967cd3b 100755 --- a/scripts/check-affected.sh +++ b/scripts/check-affected.sh @@ -48,10 +48,11 @@ # is covered, and scripts/check-affected-gates.test.sh fails when a gate in # the workflow's hygiene jobs is in neither column. # -# Code runs from the tree holding this script; the files checked are those of -# the current directory's repository. The pre-push hook runs a merge-base copy -# of this script against a branch, so the two differ: then every check script -# runs from the copy with CHECK_DATA_ROOT naming the branch tree, markdownlint +# Code runs from the tree holding this script, and the files checked are that +# tree's too, whatever the working directory, unless CHECK_DATA_ROOT names +# another tree. The pre-push hook runs a merge-base copy of this script with +# CHECK_DATA_ROOT naming the branch tree, so the two differ: then every check +# script runs from the copy with CHECK_DATA_ROOT passed on, markdownlint # reads only the copy's configs (a branch config can load a custom rule, which # is code), and checks that run the branch's own code skip: --tests, changed-skills, and the # validate-plugins steps that report `SKIP` themselves. @@ -64,7 +65,8 @@ # A missing tool, a failed fetch, or a check that exits 2 (could not run) is a # SKIP with its reason, never a failure. Exit 0 every check passed or skipped; # 1 a check failed, with each failure and its reproduce command on stderr; 2 -# usage or a git state the changed set cannot be read from. That is the +# usage or a git state the changed set cannot be read from, an empty changed +# set while HEAD is not the base among them. That is the # check-script family contract stated in README.md. set -uo pipefail @@ -94,11 +96,11 @@ while (($#)); do done tools="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)" || exit 2 -if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then - echo "check-affected: not inside a git work tree" >&2 +top="$(git -C "${CHECK_DATA_ROOT:-$tools}" rev-parse --show-toplevel 2>/dev/null)" || { + echo "check-affected: ${CHECK_DATA_ROOT:-$tools} is not inside a git work tree" >&2 exit 2 -fi -cd "$(git rev-parse --show-toplevel)" || exit 2 +} +cd "$top" || exit 2 if ((uncommitted)); then # The scratch repository has no remote, so the base is fetched and resolved @@ -134,8 +136,13 @@ if ((uncommitted)); then printf '%s\0' "${paths[@]}" | git add -A --pathspec-from-file=- --pathspec-file-nul || exit 2 fi tree="$(git write-tree)" || exit 2 - commit="$(git -c user.name=check-affected -c user.email=check-affected@localhost \ - commit-tree "$tree" -p HEAD -m 'check-affected: uncommitted work')" || exit 2 + # No edits: the snapshot is HEAD itself, so a clean tree is HEAD at its base. + if [[ "$tree" == "$(git rev-parse "HEAD^{tree}")" ]]; then + commit="$(git rev-parse HEAD)" || exit 2 + else + commit="$(git -c user.name=check-affected -c user.email=check-affected@localhost \ + commit-tree "$tree" -p HEAD -m 'check-affected: uncommitted work')" || exit 2 + fi unset GIT_INDEX_FILE GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES git -c init.defaultBranch=snapshot init --quiet --template= "$snap/tree" || exit 2 printf '%s\n' "$snap/objects" "$objects" >"$snap/tree/.git/objects/info/alternates" || exit 2 @@ -145,7 +152,7 @@ if ((uncommitted)); then args=(--base "$base" --no-fetch) ((run_tests)) && args+=(--tests) rc=0 - (cd "$snap/tree" && bash "$tools/scripts/check-affected.sh" "${args[@]}") || rc=$? + CHECK_DATA_ROOT="$snap/tree" bash "$tools/scripts/check-affected.sh" "${args[@]}" || rc=$? exit "$rc" fi base="${base:-origin/main}" @@ -209,6 +216,12 @@ if ! all_raw="$(git diff --name-only "$merge_base" HEAD)" || fi mapfile -t changed <<<"$changed_raw" [[ -n "${changed[0]-}" ]] || changed=() +# Nothing changed is a pass only when HEAD is the base; otherwise the run read +# the wrong tree or range, and passing would check nothing. +if [[ -z "$all_raw" && "$(git rev-list --count "$base...HEAD")" != 0 ]]; then + echo "check-affected: no changed files in $top since $merge_base, yet HEAD is not $base, so nothing would be checked. Run the check-affected.sh inside the tree you mean to check, or name that tree in CHECK_DATA_ROOT." >&2 + exit 2 +fi if ((${#changed[@]})) && ! git diff --quiet HEAD -- "${changed[@]}"; then echo "check-affected: SKIP every check (uncommitted edits to changed files; the checks read the working tree, a push sends HEAD, and CI checks the pushed commit). Commit or stash them, then rerun." >&2 diff --git a/scripts/check-affected.test.sh b/scripts/check-affected.test.sh index f34e234c36..a33cd742c5 100755 --- a/scripts/check-affected.test.sh +++ b/scripts/check-affected.test.sh @@ -84,6 +84,45 @@ check "every hygiene gate the runner does not run is named CI only" \ check "a gate the runner runs is not named CI only" \ '[[ $ERR != *"CI only: markdown "* && $ERR != *"CI only: spoke-plugin-root "* && $ERR != *"CI only: typos "* ]]' +# The tree checked is the one holding the script, whatever the working +# directory: run by path from another checkout whose HEAD is at its base (a main +# checkout beside a worktree), it still lists the branch's one change, notes.txt. +other="$tmp/other" +git_test_config "$tmp" clone -q --branch base "$repo" "$other" +git_test_config "$other" update-ref refs/remotes/origin/main "$(git_test_config "$repo" rev-parse HEAD)" +# run_from : runs the fixture repo's script from ; sets RC, OUT, ERR. +run_from() { + local dir="$1" + shift + RC=0 + OUT="$(cd "$dir" && bash "$repo/scripts/check-affected.sh" "$@" 2>"$tmp/err")" || RC=$? + ERR="$(cat "$tmp/err")" +} +run_from "$other" --base base --tests +check "run from another checkout, it lists the script's own branch's change" \ + '[[ $RC == 0 && $OUT == *"over 1 changed files"* && $ERR != *"SKIP tests"* ]]' +git_test_config "$repo" update-ref refs/remotes/origin/main "$(git_test_config "$repo" rev-parse base)" +run_from "$other" --base origin/main --no-fetch +check "an explicit --base run from another checkout still lists the change" \ + '[[ $RC == 0 && $OUT == *"over 1 changed files"* ]]' +git_test_config "$repo" update-ref -d refs/remotes/origin/main + +# An empty changed set while HEAD is not the base means the wrong tree or range +# was read, never a pass: HEAD behind the base, and commits that change nothing. +RC=0 +ERR="$(cd "$other" && bash scripts/check-affected.sh --base origin/main --no-fetch 2>&1)" || RC=$? +check "HEAD behind its base with no changed files exits non-zero with a message" \ + '[[ $RC != 0 && $ERR == *"no changed files"* && $ERR != *"passed"* ]]' +git_test_config "$other" commit -q --allow-empty -m empty +RC=0 +ERR="$(cd "$other" && bash scripts/check-affected.sh --base origin/base --no-fetch 2>&1)" || RC=$? +check "commits beyond the base that change no file exit non-zero with a message" \ + '[[ $RC != 0 && $ERR == *"no changed files"* && $ERR != *"passed"* ]]' +git_test_config "$other" reset -q --hard origin/base +RC=0 +OUT="$(cd "$other" && bash scripts/check-affected.sh --base base --no-fetch 2>/dev/null)" || RC=$? +check "HEAD at its base passes with nothing to check" '[[ $RC == 0 && $OUT == *"over 0 changed files"* ]]' + run STUB_TYPOS=2 -- --base base check "a typos exit of 2 is a finding, not a check that could not run" \ '[[ $RC == 1 && $ERR == *"FAIL typos"* && $ERR != *"SKIP typos"* ]]' @@ -168,9 +207,10 @@ run PATH="$tmp/bin:$PATH" -- check "a failed fetch skips the stale-base check only" \ '[[ $RC == 0 && $ERR == *"SKIP stale-base-overlap (git fetch of origin/main failed)"* && $OUT$ERR == *"validate-plugins (no plugin changed)"* ]]' -# Run from another tree (the pre-push hook's merge-base copy), every check -# script comes from that tree and is told which tree to read; the checked -# tree's own scripts and its test code never run. +# A copy outside the checked tree (the pre-push hook's merge-base copy) checks +# the tree CHECK_DATA_ROOT names: every check script comes from the copy and is +# told which tree to read; the checked tree's own scripts and its test code +# never run. tools="$tmp/tools" mkdir -p "$tools/scripts" cp "$SELF_DIR/check-affected.sh" "$tools/scripts/" @@ -183,7 +223,7 @@ printf 'import os, sys\nsys.stderr.write("copy of sync-plugin-options-docs read cp "$SELF_DIR/check-affected-gates.txt" "$tools/scripts/" chmod +x "$tools"/scripts/*.sh RC=0 -ERR="$(cd "$repo" && bash "$tools/scripts/check-affected.sh" --base base --tests 2>&1)" || RC=$? +ERR="$(cd "$tmp" && CHECK_DATA_ROOT="$repo" bash "$tools/scripts/check-affected.sh" --base base --tests 2>&1)" || RC=$? check "a copy run elsewhere runs its own check scripts against the checked tree" \ '[[ $RC == 1 && $ERR == *"copy of check-shell-portability read $repo"* ]]' check "a copy run elsewhere points the spoke, options-docs and changelog-parity checks at the checked tree" \ @@ -196,7 +236,7 @@ printf 'x\n' >"$repo/plugins/demo/skills/s/notes.txt" git_test_config "$repo" add -A git_test_config "$repo" commit -q -m skill RC=0 -ERR="$(cd "$repo" && bash "$tools/scripts/check-affected.sh" --base base 2>&1)" || RC=$? +ERR="$(cd "$tmp" && CHECK_DATA_ROOT="$repo" bash "$tools/scripts/check-affected.sh" --base base 2>&1)" || RC=$? check "a copy run elsewhere skips changed-skills, which runs the checked tree's skill-quality code" \ '[[ $ERR == *"SKIP changed-skills (runs the checked tree"* && $ERR != *"copy of check-changed-skills"* ]]' git_test_config "$repo" reset -q --hard HEAD~1 @@ -210,13 +250,13 @@ if command -v markdownlint-cli2 >/dev/null 2>&1; then git_test_config "$repo" add -A git_test_config "$repo" commit -q -m docs RC=0 - ERR="$(cd "$repo" && bash "$tools/scripts/check-affected.sh" --base base 2>&1)" || RC=$? + ERR="$(cd "$tmp" && CHECK_DATA_ROOT="$repo" bash "$tools/scripts/check-affected.sh" --base base 2>&1)" || RC=$? check "a copy run elsewhere never loads the checked tree's markdownlint config" \ '[[ ! -e $tmp/md-payload && $ERR != *"SKIP markdown"* ]]' printf '# Title\nText.\n' >"$repo/docs/a.md" git_test_config "$repo" commit -q -am bad-md RC=0 - ERR="$(cd "$repo" && bash "$tools/scripts/check-affected.sh" --base base 2>&1)" || RC=$? + ERR="$(cd "$tmp" && CHECK_DATA_ROOT="$repo" bash "$tools/scripts/check-affected.sh" --base base 2>&1)" || RC=$? check "a copy run elsewhere still lints the checked tree's markdown" \ '[[ $RC == 1 && $ERR == *"FAIL markdown"* && $ERR == *"MD022"* ]]' else @@ -237,6 +277,11 @@ printf '#!/usr/bin/env bash\necho hi\n' >"$repo/new-tool" run -- --uncommitted check "--uncommitted checks an untracked #! file's mode on disk" \ '[[ $RC == 1 && $ERR == *"FAIL exec-bit"* && $ERR == *"new-tool starts with #!"* ]]' +# The issue lane's wrapper sets CHECK_DATA_ROOT to the checkout; the snapshot, +# not the checkout's committed state, is still what gets checked. +run CHECK_DATA_ROOT="$repo" -- --uncommitted +check "--uncommitted under the lane's CHECK_DATA_ROOT still checks the work tree" \ + '[[ $RC == 1 && $ERR == *"new-tool starts with #!"* ]]' chmod +x "$repo/new-tool" run -- --uncommitted check "--uncommitted passes once the file is executable on disk" \ @@ -286,6 +331,8 @@ else ok "SKIP --uncommitted shellcheck case (no shellcheck with --rcfile here)" fi check "--uncommitted leaves HEAD, the index and the worktree list as they were" '[[ $(state) == "$before" ]]' +run -- --uncommitted +check "--uncommitted with no edits passes with nothing to check" '[[ $RC == 0 && $OUT == *"over 0 changed files"* ]]' run -- --bogus check "an unknown flag is a usage error" '[[ $RC == 2 ]]' From e51d6e55ffe8e11c91a7ef1febd8fb36d0ec22e3 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:44:47 -0400 Subject: [PATCH 2/2] style(repo): reorder the push hook's short-flag glob so typos passes typos 1.51 reads `[nd]` as a misspelling of "and" and the pre-push check blocks any branch that touches the hook; `[dn]` matches the same flags. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/check-affected-pre-push.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/hooks/check-affected-pre-push.sh b/.claude/hooks/check-affected-pre-push.sh index f781af9245..c7b2435424 100755 --- a/.claude/hooks/check-affected-pre-push.sh +++ b/.claude/hooks/check-affected-pre-push.sh @@ -177,7 +177,7 @@ segment_push_dir() { w="$1" shift case "$w" in - --delete | --dry-run | -[nd]* | -[!-]*[nd]*) return 1 ;; + --delete | --dry-run | -[dn]* | -[!-]*[dn]*) return 1 ;; --tags) tags=1 ;; -o | --push-option | --repo | --receive-pack | --exec) shift ;; -*) ;;