diff --git a/.claude/hooks/agent-bash-gate.sh b/.claude/hooks/agent-bash-gate.sh index 1c248882..632d329d 100755 --- a/.claude/hooks/agent-bash-gate.sh +++ b/.claude/hooks/agent-bash-gate.sh @@ -1,11 +1,13 @@ #!/usr/bin/env bash # Agent PR workflow gate (PreToolUse Bash). Catches accidental violations of # rules that have no human analog: -# - gh pr create without --draft +# - gh pr create (or its alias gh pr new) without --draft, or with a title +# the PR-title lint rejects # - gh pr ready -# - gh pr edit --add-reviewer / --add-assignee -# - gh api .../requested_reviewers (write verbs) +# - gh pr edit --add-reviewer / --add-assignee, and gh pr create / new +# --reviewer / --assignee (-r / -a) # - gh pr review --approve / --request-changes +# - the gh api forms of these, REST and GraphQL, and of a merge # - git push of a commit missing any pre-push review marker # (one per reviewer in scripts/pre-push-reviewers.sh — code, docs, …) # @@ -36,64 +38,290 @@ cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // ""' 2>/dev/null) \ || block "Could not parse hook payload as JSON." [ -z "$cmd" ] && exit 0 -# Strip quoted segments so commands that mention a blocked pattern in a -# string don't false-positive. Doesn't cover heredocs — pass long bodies via -# `-F ` if needed. -stripped=$(printf '%s' "$cmd" | sed -E "s/'[^']*'//g; s/\"[^\"]*\"//g") - # The gh checks resolve repo scripts from the session's checkout; the push # check resolves everything from the worktree the push runs in. project_dir="${CLAUDE_PROJECT_DIR:-.}" -# gh pr create requires --draft. -if grep -qE '(^|[[:space:];|&]+)gh[[:space:]]+pr[[:space:]]+create\b' <<<"$stripped"; then - grep -qE '(^|[[:space:]])(--draft|-d)\b' <<<"$stripped" \ - || block "Agent-opened PRs must use --draft. Only humans publish ready-for-review PRs." -fi +# ── gh: the PR actions that are humans-only, and the PR title ──────────────── +# +# gate_gh runs from the bottom of this file, once the tokenizer below exists. +# Like the push check it reads the commands the line runs, so a gh command that +# is only mentioned, in a quoted string, a heredoc body or a comment +# (`python3 - <<'EOF'` with `# gh pr ready` in it), doesn't count, unless a +# shell runs it: a $(…), `bash -c '…'`, `eval`, or a heredoc or here-string a +# shell reads, directly or through a pipe; that code, as written, quotes and +# heredoc bodies included, is tokenized and judged the same way. Each +# gh command is judged on its own words: a gh api call by its endpoint (its +# first argument that isn't a flag) and its fields, never by text elsewhere on +# the line or inside a field's value, so a comment body that names an endpoint +# is just a body. +# +# gh pr merge is not checked here: it is denied in .claude/settings.json for +# Claude's own commands, and a person merges with it in shell mode (`!`), which +# the docs don't say whether this hook sees. Its API forms, below, are blocked. -# gh pr create / gh pr edit --title: validate the title against the SAME -# Conventional-Commits rule the required `CI` check's `PR title` job enforces -# (scripts/lint-pr-title.sh is the shared rule) — so a too-long or wrong-format -# title is caught locally BEFORE the PR exists, not after the required check -# fails. Extract the quoted --title/-t value from the ORIGINAL command (the -# `stripped` copy has quotes removed). Fail-open: if no title is parseable -# (--fill, interactive, unquoted) or the validator is missing, fall through to -# the CI check rather than block a create we can't confidently judge. -if grep -qE '(^|[[:space:];|&]+)gh[[:space:]]+pr[[:space:]]+(create|edit)\b' <<<"$stripped"; then - pr_title=$(printf '%s' "$cmd" | sed -nE 's/.*(--title|-t)[[:space:]]+"([^"]*)".*/\2/p') - [ -z "$pr_title" ] && pr_title=$(printf '%s' "$cmd" | sed -nE "s/.*(--title|-t)[[:space:]]+'([^']*)'.*/\2/p") - if [ -n "$pr_title" ] && [ -x "$project_dir/scripts/lint-pr-title.sh" ]; then - if ! reason=$("$project_dir/scripts/lint-pr-title.sh" "$pr_title" 2>&1); then - block "$reason" - fi - fi -fi +# lint_title : validate a PR title against the SAME Conventional-Commits +# rule the required `CI` check's `PR title` job enforces (scripts/lint-pr-title.sh +# is the shared rule) — so a too-long or wrong-format title is caught locally +# BEFORE the PR exists, not after the required check fails. Fail-open: with no +# title to read (--fill, interactive, computed) or no validator, fall through +# to the CI check rather than block a create we can't confidently judge. +lint_title() { + local reason + [ -n "$1" ] && [ -x "$project_dir/scripts/lint-pr-title.sh" ] || return 0 + reason=$("$project_dir/scripts/lint-pr-title.sh" "$1" 2>&1) || block "$reason" +} -# gh pr ready is humans-only. -if grep -qE '(^|[[:space:];|&]+)gh[[:space:]]+pr[[:space:]]+ready\b' <<<"$stripped"; then - block "Only humans flip drafts to ready-for-review. Ask the user." -fi +msg_draft="Agent-opened PRs must use --draft. Only humans publish ready-for-review PRs." +msg_ready="Only humans flip drafts to ready-for-review. Ask the user." +msg_reviewers="Adding/removing reviewers is humans-only. Re-trigger bot reviewers via PR comment mention (e.g. @coderabbitai review)." +msg_reviewers_api="Reviewer-write requests are humans-only. Re-trigger bot reviewers via PR comment mention." +msg_approve="Only humans approve PRs." +msg_changes="Agents post inline review comments instead of --request-changes." +merge_msg="Agents don't merge PRs; a person does. Ask the user." -# gh pr edit --add-reviewer / --add-assignee is humans-only. -if grep -qE '(^|[[:space:];|&]+)gh[[:space:]]+pr[[:space:]]+edit\b' <<<"$stripped" \ - && grep -qE '(^|[[:space:]])--(add|remove)-(reviewer|assignee)\b' <<<"$stripped"; then - block "Adding/removing reviewers is humans-only. Re-trigger bot reviewers via PR comment mention (e.g. @coderabbitai review)." -fi +# truthy <--flag=value>: the value turns a boolean flag on, as gh's flag +# parser reads it. +truthy() { + case ${1#*=} in 1 | t | T | true | TRUE | True) return 0 ;; esac + return 1 +} -# gh api .../requested_reviewers write verbs (API form of --add-reviewer). -if grep -qE '(^|[[:space:];|&]+)gh[[:space:]]+api\b' <<<"$stripped" \ - && grep -qE 'requested_reviewers' <<<"$stripped" \ - && grep -qE '(-X[[:space:]]*(POST|PUT|PATCH)|--method[[:space:]]*(POST|PUT|PATCH)|[[:space:]]-f[[:space:]]+reviewers=|[[:space:]]-F[[:space:]]+reviewers=)' <<<"$stripped"; then - block "Reviewer-write requests are humans-only. Re-trigger bot reviewers via PR comment mention." -fi +# gh_pr <i> <end>: judge `gh pr <TK_VAL[i]> …`, its words up to token <end>. +gh_pr() { + local i=$1 e=$2 j w sub=${TK_VAL[$1]} draft=0 approve=0 changes=0 reviewer=0 people=0 title="" + for ((j = i + 1; j < e; j++)); do + w=${TK_VAL[j]} + case $w in + --draft | -d) draft=1 ;; + --draft=*) truthy "$w" && draft=1 ;; + --approve) approve=1 ;; + -a) approve=1; people=1 ;; + --approve=*) truthy "$w" && approve=1 ;; + --request-changes) changes=1 ;; + -r) changes=1; people=1 ;; + --request-changes=*) truthy "$w" && changes=1 ;; + # `create` reads these as people to request or assign; gh also takes the value glued (-rname, -r=name). + --reviewer | --reviewer=* | --assignee | --assignee=* | -r?* | -a?*) people=1 ;; + --add-reviewer | --add-reviewer=* | --remove-reviewer | --remove-reviewer=*) reviewer=1 ;; + --add-assignee | --add-assignee=* | --remove-assignee | --remove-assignee=*) reviewer=1 ;; + --title | -t) [ $((j + 1)) -lt "$e" ] && [ "${TK_DYN[j + 1]}" = 0 ] && title=${TK_VAL[j + 1]} ;; + --title=*) [ "${TK_DYN[j]}" = 0 ] && title=${w#--title=} ;; + esac + done + case $sub in + create | new) + [ "$people" = 0 ] || block "$msg_reviewers" + [ "$draft" = 1 ] || block "$msg_draft" + lint_title "$title" ;; + edit) + [ "$reviewer" = 0 ] || block "$msg_reviewers" + lint_title "$title" ;; + ready) block "$msg_ready" ;; + review) + [ "$approve" = 0 ] || block "$msg_approve" + [ "$changes" = 0 ] || block "$msg_changes" ;; + esac + return 0 +} -# gh pr review --approve / --request-changes are humans-only. -if grep -qE '(^|[[:space:];|&]+)gh[[:space:]]+pr[[:space:]]+review\b' <<<"$stripped"; then - grep -qE '(^|[[:space:]])(--approve|-a)\b' <<<"$stripped" \ - && block "Only humans approve PRs." - grep -qE '(^|[[:space:]])(--request-changes|-r)\b' <<<"$stripped" \ - && block "Agents post inline review comments instead of --request-changes." -fi +# gh_api <i> <end>: judge `gh api …`, its words from token <i> up to <end>. +# The endpoint is its first word that isn't a flag or a flag's value; a field +# is the value of -f, -F, --field or --raw-field. A call writes when it names a +# method other than GET or, naming none, sends a field or --input, which makes +# gh POST. A field sent from a file (`-F key=@file`, --input) isn't read. +gh_api() { + local i=$1 e=$2 j w f ep="" method="" fields=0 write=0 path + local draft="" title="" event="" query="" qdyn=0 values="" + for ((j = i; j < e; j++)); do + w=${TK_VAL[j]} + f="" + case $w in + -X | --method) [ $((j + 1)) -lt "$e" ] && { j=$((j + 1)); method=${TK_VAL[j]}; } ;; + -X?*) method=${w#-X} ;; + --method=*) method=${w#--method=} ;; + -f | -F | --field | --raw-field) [ $((j + 1)) -lt "$e" ] && { j=$((j + 1)); f=${TK_VAL[j]}; fields=1; } ;; + -f?* | -F?*) f=${w#-?}; fields=1 ;; + --field=* | --raw-field=*) f=${w#*=}; fields=1 ;; + --input) j=$((j + 1)); fields=1 ;; + --input=*) fields=1 ;; + -H | --header | -q | --jq | -t | --template | -p | --preview | --hostname | --cache) j=$((j + 1)) ;; + -*) ;; + *) [ -n "$ep" ] || ep=$w ;; + esac + [ -n "$f" ] || continue + case ${f#*=} in @*) continue ;; esac # its value is a file's contents + values+=" ${f#*=}" + case $f in + draft=*) draft=${f#draft=} ;; + title=*) [ "${TK_DYN[j]}" = 0 ] && title=${f#title=} ;; + event=*) event=${f#event=} ;; + query=*) query=${f#query=}; [ "${TK_DYN[j]}" = 0 ] || qdyn=1 ;; + esac + done + # A computed query (`-f query="$(cat <<'EOF' … EOF)"`) is matched against + # the code that computes it, heredoc bodies included. + [ "$qdyn" = 0 ] || query=$GH_CODE + if [ -n "$method" ]; then + case $method in [gG][eE][tT]) ;; *) write=1 ;; esac + else + write=$fields + fi + path=${ep#https://api.github.com} + path=${path#/} + path=${path%%\?*} + path=${path%/} + if [ "$path" = graphql ]; then + case $query in *markPullRequestReadyForReview*) block "$msg_ready" ;; esac + case $query in *mergePullRequest* | *enablePullRequestAutoMerge*) block "$merge_msg" ;; esac + case $query in *requestReviews*) block "$msg_reviewers_api" ;; esac + case $query in + *addPullRequestReview* | *submitPullRequestReview*) + case "$query$values" in *APPROVE*) block "$msg_approve" ;; esac + case "$query$values" in *REQUEST_CHANGES*) block "$msg_changes" ;; esac ;; + esac + case $query in + *createPullRequest*) + [[ $query =~ draft:[[:space:]]*true || $draft == true ]] \ + || block "Agent-opened PRs must be drafts; in GraphQL, send draft: true. Only humans publish ready-for-review PRs." ;; + esac + return 0 + fi + if [ "$write" = 1 ]; then + case $path in + repos/*/pulls/*/requested_reviewers) block "$msg_reviewers_api" ;; + repos/*/pulls/*/merge) block "$merge_msg" ;; + repos/*/pulls) + [ "$draft" = true ] \ + || block "Agent-opened PRs must be drafts; through the API, send -F draft=true. Only humans publish ready-for-review PRs." + lint_title "$title" ;; + repos/*/pulls/*/*) ;; + repos/*/pulls/*) lint_title "$title" ;; + esac + fi + case $path in + repos/*/pulls/*/reviews | repos/*/pulls/*/reviews/*/events) + case $event in + [aA][pP][pP][rR][oO][vV][eE]) block "$msg_approve" ;; + [rR][eE][qQ][uU][eE][sS][tT]_[cC][hH][aA][nN][gG][eE][sS]) block "$msg_changes" ;; + esac ;; + esac + return 0 +} + +# shell_code_at <i> <end>: SHELL_CODE = the token index of the code a shell +# whose arguments start at token <i> runs with -c (its first argument after +# the options), or -1 when it isn't given -c. Steps over options and the value +# of -o, +o, -O, +O, --rcfile and --init-file; a short-option cluster that +# contains c (-c, -lc, -euc) counts as -c, a long option (--norc) doesn't. +shell_code_at() { + local m=$1 e=$2 c=0 + SHELL_CODE=-1 + while [ "$m" -lt "$e" ]; do + case ${TK_VAL[m]} in + --) m=$((m + 1)); break ;; + --rcfile | --init-file) m=$((m + 1)) ;; + --*) ;; + [-+]*[oO]*) + case ${TK_VAL[m]} in -*c*) c=1 ;; esac + m=$((m + 1)) ;; + -*c*) c=1 ;; + [-+]?*) ;; + *) break ;; + esac + m=$((m + 1)) + done + if [ "$c" = 1 ] && [ "$m" -lt "$e" ]; then SHELL_CODE=$m; fi +} + +# gh_scan: judge every gh pr and gh api command in the current tokens, and +# queue in GH_QUEUE the code a shell will run: each $(…) and `…`, the code a +# shell is handed with -c or eval, and a heredoc or here-string a shell reads, +# directly or through a pipe. +gh_scan() { + local i j k m n=${#TK_VAL[@]} w line="" start=0 s + local -a lines=() starts=() ends=() + for ((i = 0; i <= n; i++)); do + if [ "$i" -eq "$n" ] || [ "${TK_OP[i]}" = 1 ]; then + lines+=("$line"); starts+=("$start"); ends+=("$i") + line=""; start=$((i + 1)) + continue + fi + w=${TK_VAL[i]} + case $w in *[[:space:]]*) w=Q ;; esac + [ "${TK_DYN[i]}" = 0 ] || w=Q + line+=" $w" + done + for ((k = 0; k < ${#lines[@]}; k++)); do + for ((j = starts[k]; j < ends[k]; j++)); do + w=${TK_VAL[j]##*/} + if [ "$w" = gh ] && [ "${TK_DYN[j]}" = 0 ]; then + # gh takes -R/--repo before the command too: `gh -R o/r pr ready 12`. + m=$((j + 1)) + while [ "$m" -lt "${ends[k]}" ]; do + case ${TK_VAL[m]} in + -R | --repo) m=$((m + 2)) ;; + -R?* | --repo=*) m=$((m + 1)) ;; + *) break ;; + esac + done + if [ "$m" -lt "${ends[k]}" ]; then + case ${TK_VAL[m]} in + pr) [ $((m + 1)) -lt "${ends[k]}" ] && gh_pr $((m + 1)) "${ends[k]}" ;; + api) gh_api $((m + 1)) "${ends[k]}" ;; + esac + fi + break + fi + # Code handed to a shell: `bash -c '…'` (or -lc, -e -c, …), or eval's words. + case $w in + sh | bash | dash | zsh | ksh) + shell_code_at $((j + 1)) "${ends[k]}" + [ "$SHELL_CODE" -lt 0 ] || GH_QUEUE+=("${TK_VAL[SHELL_CODE]}") ;; + eval) + s="" + for ((i = j + 1; i < ends[k]; i++)); do s+=" ${TK_VAL[i]}"; done + GH_QUEUE+=("$s") ;; + esac + done + done + for ((j = 0; j < ${#TK_CODE[@]}; j++)); do + GH_QUEUE+=("${TK_CODE[j]}") + done + for ((j = 0; j < ${#TK_IN[@]}; j++)); do + for ((k = 0; k < ${#lines[@]}; k++)); do + [ "${starts[k]}" = "${TK_IN_AT[j]}" ] || continue + if [[ ${lines[k]} =~ $shell_re ]] \ + || { [ $((k + 1)) -lt "${#lines[@]}" ] && [[ ${TK_VAL[ends[k]]} == '|'* ]] && [[ ${lines[k + 1]} =~ $shell_re ]]; }; then + GH_QUEUE+=("${TK_IN[j]}") + fi + break + done + done +} + +# gate_gh: judge the line, then each piece of code it hands a shell, in turn. +# It follows 64 pieces, a bound on code that nests itself; past that, any piece +# left that mentions a gh pr or gh api command is refused, unjudged. So is a +# line it can't parse that mentions one: it can't tell what runs. +gate_gh() { + local q + grep -qE '(^|[^[:alnum:]_-])gh[[:space:]]' <<<"$cmd" || return 0 + GH_QUEUE=("$cmd") + for ((q = 0; q < ${#GH_QUEUE[@]} && q < 64; q++)); do + GH_CODE=${GH_QUEUE[q]} + if tokenize "$GH_CODE"; then + gh_scan + elif grep -qE "$gh_cmd_re" <<<"$GH_CODE"; then + block "can't parse this command (an unterminated quote or substitution?), so can't tell what it does to a PR. Fix the quoting, or run the gh command on its own." + fi + done + for (( ; q < ${#GH_QUEUE[@]}; q++)); do + grep -qE "$gh_cmd_re" <<<"${GH_QUEUE[q]}" \ + && block "this command nests more code than the gate follows (64 pieces), and some of what's left runs gh. Split it into separate commands." + done + return 0 +} # ── git push: the commit each refspec publishes needs a review marker ──────── # @@ -138,7 +366,9 @@ reviewers_script="scripts/pre-push-reviewers.sh" # TK_OP, TK_IN / TK_IN_AT with each heredoc body and here-string and the token # index of the simple command that reads it, and TK_SUBS with the text of every # $(…) and `…`, wherever its word went (an argument, a here-string, a redirect -# target), each quoted part inside a $(…) wrapped in \002…\003; returns 1 on an +# target), each quoted part inside a $(…) wrapped in \002…\003, and TK_CODE +# with the code inside each one exactly as written, quotes and heredoc bodies +# included, for the gh checks to tokenize in turn; returns 1 on an # unterminated quote or substitution. tk_flush() { @@ -183,7 +413,7 @@ tk_ansi() { } tk_dquote() { - local c c2 + local c c2 rest part _inw=1; _i=$((_i + 1)) [ "$_insub" = 0 ] || _w+=$'\002' while [ "$_i" -lt "$_n" ]; do @@ -199,20 +429,26 @@ tk_dquote() { _i=$((_i + 2)) ;; '$') tk_dollar || return 1 ;; '`') tk_backtick || return 1 ;; - *) _w+=$c; _i=$((_i + 1)) ;; + *) + # Everything up to the next character with a meaning here, in one step: + # a PR body is often tens of KB of plain text in double quotes. + rest=${_s:_i} + part=${rest%%[\"\\\$\`]*} + _w+=$part; _i=$((_i + ${#part})) ;; esac done return 1 } tk_dollar() { - local rest part s0=${#_w} + local rest part s0=${#_w} start=$_i _inw=1; _dyn=1 case ${_s:_i+1:1} in '(') _w+='$'; _i=$((_i + 1)); _insub=$((_insub + 1)) tk_paren || return 1 - _insub=$((_insub - 1)); TK_SUBS+=("${_w:s0}") ;; + _insub=$((_insub - 1)); TK_SUBS+=("${_w:s0}") + TK_CODE+=("${_s:start+2:_i-start-3}") ;; '{') rest=${_s:_i} case $rest in *'}'*) ;; *) return 1 ;; esac @@ -225,12 +461,15 @@ tk_dollar() { } tk_backtick() { - local c s0=${#_w} + local c s0=${#_w} start=$_i _inw=1; _dyn=1; _w+='`'; _i=$((_i + 1)) while [ "$_i" -lt "$_n" ]; do c=${_s:_i:1} case $c in - '`') _w+=$c; _i=$((_i + 1)); TK_SUBS+=("${_w:s0}"); return 0 ;; + '`') + _w+=$c; _i=$((_i + 1)); TK_SUBS+=("${_w:s0}") + TK_CODE+=("${_s:start+1:_i-start-2}") + return 0 ;; \\) _w+=${_s:_i:2}; _i=$((_i + 2)) ;; *) _w+=$c; _i=$((_i + 1)) ;; esac @@ -340,7 +579,7 @@ tokenize() { local LC_ALL=C c c2 rest line _s=$1; _n=${#1}; _i=0 _w=""; _inw=0; _dyn=0; _skip=0; _cmd0=0; _insub=0 - TK_VAL=(); TK_DYN=(); TK_OP=(); TK_IN=(); TK_IN_AT=(); TK_SUBS=(); _hd_delim=(); _hd_strip=(); _hd_at=() + TK_VAL=(); TK_DYN=(); TK_OP=(); TK_IN=(); TK_IN_AT=(); TK_SUBS=(); TK_CODE=(); _hd_delim=(); _hd_strip=(); _hd_at=() while [ "$_i" -lt "$_n" ]; do c=${_s:_i:1} case $c in @@ -697,6 +936,10 @@ git_re='(^|[[:space:];|&(`])git([[:space:]]+-[^[:space:]]+([[:space:]]+[^-[:spac # shellcheck disable=SC2016 end_re='([[:space:]]|$|[;|&)`])' push_re=${git_re}push$end_re +# shellcheck disable=SC2016 +gh_re='(^|[[:space:];|&(`])gh[[:space:]]+' +# A gh pr or gh api command, -R/--repo before it allowed, anywhere in a text. +gh_cmd_re=${gh_re}'((-R|--repo)(=|[[:space:]]*)[^[:space:]]+[[:space:]]+)*(pr|api)'$end_re head_re=${git_re}'(commit|merge|rebase|reset|checkout|switch|cherry-pick|revert|am|pull)'$end_re # shellcheck disable=SC2016 shell_re='(^|[[:space:];|&(`])(eval|([^[:space:];|&]*/)?(ba|z|da|k)?sh)([[:space:]<]|$)' @@ -760,6 +1003,8 @@ unfollowed_push() { return 1 } +gate_gh + # Cheap filter first, so most commands are never tokenized: quoted strings on a # line become a placeholder word (`git -C "<path>" stash push` reads as a # stash). It may still match a push that is only mentioned, say in a heredoc diff --git a/.claude/hooks/hook-commands.test.sh b/.claude/hooks/hook-commands.test.sh new file mode 100755 index 00000000..280fc93f --- /dev/null +++ b/.claude/hooks/hook-commands.test.sh @@ -0,0 +1,321 @@ +#!/usr/bin/env bash +# Behavioral test for the hook commands in .claude/settings.json. Each command +# runs its script from $CLAUDE_PROJECT_DIR/.claude/hooks/ when the script is +# there and executable, exactly as a bare path would. When it isn't (the +# session's checkout was removed, say), the shell would exit 127, which Claude +# Code treats as a non-blocking error, so the push gate would stop gating +# (#787). Instead the PreToolUse command blocks anything that looks like a push +# or a PR change, the SubagentStop command reports the missing script without +# exit 2 (which would keep the subagent running), and the PostToolUse commands +# report it with exit 2, which shows Claude the message after the edit. +# +# Every command is read from the real settings.json and run the way Claude +# Code runs a shell-form hook: `sh -c`, the event JSON on stdin, +# CLAUDE_PROJECT_DIR in the environment; under dash and bash too when present. +# Run by `make verify` (target: test-hook-commands). Needs jq; no network. + +set -uo pipefail + +# The hook scripts run under whichever bash `#!/usr/bin/env bash` finds first. +# macOS ships bash 3.2 as /bin/bash, so when that is another bash the suite runs +# a second time with it first on PATH (review-gate.test.sh does the same). +if [ -z "${HOOK_TEST_BASH:-}" ]; then + self="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" + HOOK_TEST_BASH=$(command -v bash) "$BASH" "$self" + rc=$? + if [ -x /bin/bash ] && ! [ /bin/bash -ef "$(command -v bash)" ]; then + shim=$(mktemp -d) && ln -s /bin/bash "$shim/bash" || exit 1 + echo "── the hooks again under /bin/bash $(/bin/bash -c 'echo "$BASH_VERSION"')" + PATH="$shim:$PATH" HOOK_TEST_BASH=/bin/bash "$BASH" "$self" || rc=1 + rm -rf "$shim" + fi + exit "$rc" +fi + +cd "$(dirname "${BASH_SOURCE[0]}")/../.." || exit 1 # repo root (.claude/hooks/../..) +root=$PWD +settings=$root/.claude/settings.json +fails=0 + +command -v jq >/dev/null 2>&1 || { echo "hook-commands test: jq is required." >&2; exit 1; } + +scratch=$(mktemp -d) || exit 1 +trap 'rm -rf "$scratch"' EXIT +scratch=$(cd "$scratch" && pwd -P) + +ok() { printf ' ok %s\n' "$1"; } +fail() { + printf ' FAIL %s\n' "$1" >&2 + [ -n "${2:-}" ] && printf '%s\n' "$2" | sed 's/^/ /' >&2 + fails=$((fails + 1)) +} + +# commands <event>: the event's command hooks, one JSON-encoded string a line. +commands() { + jq -c --arg e "$1" '.hooks[$e][]? | .hooks[] | select(.type == "command") | .command' "$settings" +} + +# The script each event runs: the contract this test holds settings.json to. +pre=$(jq -c '.hooks.PreToolUse[] | select(.matcher == "Bash") | .hooks[] | .command' "$settings") +stop=$(commands SubagentStop) +post=$(commands PostToolUse) +[ "$(printf '%s\n' "$pre" | grep -c .)" = 1 ] || fail "settings.json has one PreToolUse Bash command" "$pre" +[ "$(printf '%s\n' "$stop" | grep -c .)" = 1 ] || fail "settings.json has one SubagentStop command" "$stop" +[ "$(printf '%s\n' "$post" | grep -c .)" = 2 ] || fail "settings.json has two PostToolUse commands" "$post" +pre=$(jq -r . <<<"$pre") +stop=$(jq -r . <<<"$stop") +post_scripts=(gofumpt-on-save.sh markdown-on-save.sh) + +shells=() +for s in sh dash bash; do + p=$(command -v "$s") && shells+=("$p") +done + +# A PATH with what the commands need but no jq. +nojq=$scratch/nojq +mkdir "$nojq" +for t in cat grep; do ln -s "$(command -v "$t")" "$nojq/$t"; done + +gone=$scratch/gone # the project directory of a session whose checkout was removed +mkdir "$gone" +noexec=$scratch/noexec # one whose hook scripts are there but not executable +mkdir -p "$noexec/.claude/hooks" +for s in agent-bash-gate.sh review-marker.sh gofumpt-on-save.sh markdown-on-save.sh; do + cp "$root/.claude/hooks/$s" "$noexec/.claude/hooks/$s" + chmod a-x "$noexec/.claude/hooks/$s" +done + +# run <shell> <command> <project-dir|-> <payload-file> [PATH]: run a hook +# command as Claude Code does, with CLAUDE_PROJECT_DIR unset for `-`; sets rc, +# out and err. +run() { + local sh=$1 cmd=$2 dir=$3 payload=$4 path=${5:-$PATH} + if [ "$dir" = - ]; then + env -u CLAUDE_PROJECT_DIR PATH="$path" "$sh" -c "$cmd" <"$payload" >"$scratch/out" 2>"$scratch/err" + else + CLAUDE_PROJECT_DIR=$dir PATH=$path "$sh" -c "$cmd" <"$payload" >"$scratch/out" 2>"$scratch/err" + fi + rc=$? + out=$(cat "$scratch/out") + err=$(cat "$scratch/err") +} + +# expect <name> <want-rc> <err-pattern> : check the last run. An empty pattern +# means stderr must be empty; a pattern led by `!` must not appear. +expect() { + local name=$1 want=$2 pat=$3 bad="" + [ "$rc" = "$want" ] || bad="exit $rc, want $want" + case $pat in + '') [ -z "$err" ] || bad="${bad:+$bad; }stderr not empty" ;; + !*) case $err in *"${pat#!}"*) bad="${bad:+$bad; }stderr has '${pat#!}'" ;; esac ;; + *) case $err in *"$pat"*) ;; *) bad="${bad:+$bad; }stderr lacks '$pat'" ;; esac ;; + esac + if [ -z "$bad" ]; then ok "$name"; else fail "$name: $bad" "$err"; fi +} + +# Payloads, shaped like Claude Code's. The Bash description mentions a push on +# purpose: only the command may decide. +bash_payload() { + jq -nc --arg c "$1" --arg cwd "$scratch" '{session_id: "t", transcript_path: "/dev/null", cwd: $cwd, + hook_event_name: "PreToolUse", tool_name: "Bash", + tool_input: {command: $c, description: "git push the branch, then gh pr create"}}' >"$scratch/payload" +} +stop_payload() { + jq -nc --arg t "$1" '{session_id: "t", transcript_path: "/dev/null", hook_event_name: "SubagentStop", + stop_hook_active: false, agent_id: "a1", agent_type: $t, agent_transcript_path: "/nonexistent", + last_assistant_message: "REVIEWED: 0000000\nVERDICT: ship_it"}' >"$scratch/payload" +} +edit_payload() { + jq -nc --arg f "$1" '{session_id: "t", transcript_path: "/dev/null", hook_event_name: "PostToolUse", + tool_name: "Edit", tool_input: {file_path: $f, old_string: "a", new_string: "b"}, + tool_response: {filePath: $f, success: true}}' >"$scratch/payload" +} + +pushes=( + 'git push' + 'git push -u origin feat' + 'cd sub && git -C ../wt push origin HEAD' + "$(printf 'git commit -qm x\ngit push')" + 'gh pr create --draft --title "fix: x"' + 'gh pr new --title "fix: x"' + 'gh pr ready 12' + 'gh pr edit 12 --add-reviewer someone' + 'gh pr merge 12 --squash' + 'gh pr review 12 --approve' + 'gh api -X POST repos/o/r/pulls/1/requested_reviewers -f reviewers[]=someone' + 'gh api repos/o/r/pulls/1/reviews -f event=APPROVE' + 'gh api repos/o/r/pulls -f head=feat -f base=main -f title="fix: x"' + 'gh api -X PUT repos/o/r/pulls/1/merge' + "gh api graphql -f query='mutation { markPullRequestReadyForReview(input: {pullRequestId: \"x\"}) { clientMutationId } }'" +) +others=( + 'ls -la' + 'git status' + 'git log --oneline -5' + 'gh pr view 12' + 'gh pr checks 12' + 'gh issue edit 5 --title x' + 'gh api repos/o/r/issues/5/comments' + 'make verify' +) + +for sh in "${shells[@]}"; do + echo "── $sh" + + # Every command in settings.json, for every event: a script that is missing, + # or there but not executable, must not end the hook the way the shell would + # (126/127, or 0), and the hook must say what is wrong. + while IFS= read -r line; do + event=$(jq -r '.[0]' <<<"$line") + cmd=$(jq -r '.[1]' <<<"$line") + jq -nc --arg e "$event" '{hook_event_name: $e, tool_name: "Bash", tool_input: {command: "git push", file_path: "x.go"}, + agent_type: "pre-push-reviewer"}' >"$scratch/payload" + for d in "$gone" "$noexec"; do + what=missing + [ "$d" = "$noexec" ] && what="not executable" + run "$sh" "$cmd" "$d" "$scratch/payload" + name="$event hook with its script $what says so (exit $rc, not 0/126/127)" + if [ "$rc" != 0 ] && [ "$rc" != 126 ] && [ "$rc" != 127 ] && [[ $err == *"is missing or not executable"* ]]; then + ok "$name" + else + fail "$name" "$err" + fi + done + done < <(jq -c '.hooks | to_entries[] | .key as $e | .value[] | .hooks[] | select(.type == "command") | [$e, .command]' "$settings") + + # PreToolUse: the gate's script is gone. + for c in "${pushes[@]}"; do + bash_payload "$c" + run "$sh" "$pre" "$gone" "$scratch/payload" + expect "PreToolUse, gate missing: blocks '${c//$'\n'/; }'" 2 "agent-bash-gate.sh is missing or not executable" + done + if [[ $err == *"restart Claude Code from a live checkout"* ]]; then + ok "PreToolUse, gate missing: says how to recover" + else + fail "PreToolUse, gate missing: says how to recover" "$err" + fi + for c in "${others[@]}"; do + bash_payload "$c" + run "$sh" "$pre" "$gone" "$scratch/payload" + expect "PreToolUse, gate missing: lets '$c' through" 0 "" + done + bash_payload 'git push' + run "$sh" "$pre" - "$scratch/payload" + expect "PreToolUse, CLAUDE_PROJECT_DIR unset: blocks a push" 2 "is missing or not executable" + bash_payload 'ls -la' + run "$sh" "$pre" "$gone" "$scratch/payload" "$nojq" + expect "PreToolUse, gate missing and no jq: blocks every command" 2 "without jq, every command is" + printf 'not json' >"$scratch/payload" + run "$sh" "$pre" "$gone" "$scratch/payload" + expect "PreToolUse, gate missing: blocks an unreadable payload" 2 "is missing or not executable" + bash_payload 'git push' + run "$sh" "$pre" "$noexec" "$scratch/payload" + expect "PreToolUse, gate not executable: blocks a push" 2 "is missing or not executable" + bash_payload 'ls -la' + run "$sh" "$pre" "$noexec" "$scratch/payload" + expect "PreToolUse, gate not executable: lets 'ls -la' through" 0 "" + + for d in "$gone" "$noexec"; do + what=missing + [ "$d" = "$noexec" ] && what="not executable" + + # SubagentStop: visible, but never exit 2 (that keeps the subagent running). + stop_payload pre-push-reviewer + run "$sh" "$stop" "$d" "$scratch/payload" + expect "SubagentStop, marker hook $what: reports it on stderr with exit 1" 1 "review-marker.sh is missing or not executable" + + # PostToolUse: exit 2 shows Claude the message; the edit already happened. + i=0 + while IFS= read -r c; do + c=$(jq -r . <<<"$c") + edit_payload "$scratch/x.go" + run "$sh" "$c" "$d" "$scratch/payload" + expect "PostToolUse, ${post_scripts[i]} $what: tells Claude with exit 2" 2 "${post_scripts[i]} is missing or not executable" + i=$((i + 1)) + done <<<"$post" + done + + # A missing script's command still reads its whole payload: a hook that + # exits without reading would leave a large write to its stdin unfinished. + head -c 300000 /dev/zero | tr '\0' x | jq -Rsc '{hook_event_name: "PostToolUse", tool_name: "Write", + tool_input: {file_path: "x.md", content: .}}' >"$scratch/big" + drain=("PreToolUse" "$pre" "SubagentStop" "$stop" "PostToolUse" "$(jq -r . <<<"${post%%$'\n'*}")") + for ((k = 0; k < ${#drain[@]}; k += 2)); do + { cat "$scratch/big" && echo finished >"$scratch/writer"; } \ + | CLAUDE_PROJECT_DIR=$gone PATH=$nojq "$sh" -c "${drain[k + 1]}" >/dev/null 2>&1 + if [ "$(cat "$scratch/writer" 2>/dev/null)" = finished ]; then + ok "${drain[k]}, script missing: reads a 300 KB payload to the end" + else + fail "${drain[k]}, script missing: reads a 300 KB payload to the end" + fi + rm -f "$scratch/writer" + done + + # A live project directory: each command runs its own script, with the + # payload byte for byte on stdin, no arguments, and nothing of its own on + # stdout or stderr; the script's exit code is the hook's. + live=$scratch/live + rm -rf "$live" + mkdir -p "$live/.claude/hooks" + for s in agent-bash-gate.sh review-marker.sh "${post_scripts[@]}"; do + # shellcheck disable=SC2016 # expanded by the stub + printf '#!/bin/sh\ncat >"$0.stdin"\nprintf "%%s %%s\\n" "$#" "$0" >"$0.argv"\necho stub-out\necho stub-err >&2\nexit 42\n' >"$live/.claude/hooks/$s" + chmod +x "$live/.claude/hooks/$s" + done + # shellcheck disable=SC2016 # a literal $(x), for the stub to receive as is + printf '{"tool_input":{"command":"git push \\"$(x)\\" \\u00e9 é \\\\ \x27q\x27"}}' >"$scratch/odd" # no trailing newline + i=0 + while IFS= read -r line; do + s=$(jq -r '.[0]' <<<"$line") + cmd=$(jq -r '.[1]' <<<"$line") + rm -f "$live/.claude/hooks/$s".stdin "$live/.claude/hooks/$s".argv + run "$sh" "$cmd" "$live" "$scratch/odd" + name="live project dir: runs $s with its stdin, argv and exit code" + if [ "$rc" = 42 ] && [ "$out" = stub-out ] && [ "$err" = stub-err ] \ + && cmp -s "$scratch/odd" "$live/.claude/hooks/$s.stdin" \ + && [ "$(cat "$live/.claude/hooks/$s.argv")" = "0 $live/.claude/hooks/$s" ]; then + ok "$name" + else + fail "$name" "exit $rc; out: $out; err: $err; argv: $(cat "$live/.claude/hooks/$s.argv" 2>&1)" + fi + i=$((i + 1)) + done < <(jq -c '.hooks | to_entries[] | .value[] | .hooks[] | select(.type == "command") | .command + | [(capture("/\\.claude/hooks/(?<s>[A-Za-z0-9_.-]+\\.sh)").s), .]' "$settings") + [ "$i" = 4 ] || fail "live project dir: ran all four commands (ran $i)" + spaced="$scratch/live project" + rm -rf "$spaced" + cp -R "$live" "$spaced" + rm -f "$spaced"/.claude/hooks/*.stdin "$spaced"/.claude/hooks/*.argv + run "$sh" "$pre" "$spaced" "$scratch/odd" + if [ "$rc" = 42 ] && cmp -s "$scratch/odd" "$spaced/.claude/hooks/agent-bash-gate.sh.stdin"; then + ok "live project dir with a space in its path: runs agent-bash-gate.sh" + else + fail "live project dir with a space in its path: runs agent-bash-gate.sh" "exit $rc; err: $err" + fi + + # This checkout: the real scripts run, not the fallback. + bash_payload 'gh pr ready 12' + run "$sh" "$pre" "$root" "$scratch/payload" + expect "this checkout: the real gate blocks gh pr ready" 2 "Only humans flip drafts" + expect "this checkout: the real gate, not the fallback" 2 "!is missing or not executable" + bash_payload 'ls -la' + run "$sh" "$pre" "$root" "$scratch/payload" + expect "this checkout: the real gate lets ls through" 0 "" + stop_payload "" + run "$sh" "$stop" "$root" "$scratch/payload" + expect "this checkout: the real marker hook ignores a non-reviewer" 0 "" + i=0 + while IFS= read -r c; do + c=$(jq -r . <<<"$c") + edit_payload "$scratch/notes.txt" + run "$sh" "$c" "$root" "$scratch/payload" + expect "this checkout: the real ${post_scripts[i]} skips a .txt file" 0 "" + i=$((i + 1)) + done <<<"$post" +done + +if [ "$fails" -gt 0 ]; then + printf '\n%d case(s) failed\n' "$fails" >&2 + exit 1 +fi +echo "hook-commands: all cases passed" diff --git a/.claude/hooks/review-gate.test.sh b/.claude/hooks/review-gate.test.sh index 7bdfe562..5d5a5744 100755 --- a/.claude/hooks/review-gate.test.sh +++ b/.claude/hooks/review-gate.test.sh @@ -9,6 +9,24 @@ # `make verify` (target: test-review-gate). Needs git and jq; no network. set -uo pipefail + +# The hooks run under whichever bash `#!/usr/bin/env bash` finds first. macOS +# ships bash 3.2 as /bin/bash, and a Mac with no newer bash on PATH runs them +# under it, so when /bin/bash is another bash the suite runs a second time with +# it first on PATH. The test itself stays on this bash either way. +if [ -z "${HOOK_TEST_BASH:-}" ]; then + self="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" + HOOK_TEST_BASH=$(command -v bash) "$BASH" "$self" + rc=$? + if [ -x /bin/bash ] && ! [ /bin/bash -ef "$(command -v bash)" ]; then + shim=$(mktemp -d) && ln -s /bin/bash "$shim/bash" || exit 1 + echo "── the hooks again under /bin/bash $(/bin/bash -c 'echo "$BASH_VERSION"')" + PATH="$shim:$PATH" HOOK_TEST_BASH=/bin/bash "$BASH" "$self" || rc=1 + rm -rf "$shim" + fi + exit "$rc" +fi + cd "$(dirname "${BASH_SOURCE[0]}")/../.." || exit 1 # repo root (.claude/hooks/../..) root=$PWD gate=$root/.claude/hooks/agent-bash-gate.sh @@ -495,6 +513,170 @@ fi CLAUDE_PROJECT_DIR=$root expect_block "gh pr create title lint" "$repo" "gh pr create --draft --title \"Bad title.\"" "PR title" expect_block "gh pr create without --draft" "$repo" "gh pr create --title \"fix: x\"" +# The alias, REST and GraphQL forms of the humans-only gh pr actions (#790). +expect_block "gh pr new without --draft" "$repo" 'gh pr new --title "fix: x"' "must use --draft" +CLAUDE_PROJECT_DIR=$root expect_block "gh pr new title lint" "$repo" 'gh pr new --draft --title "Bad title."' "PR title" +CLAUDE_PROJECT_DIR=$root expect_block "an unquoted gh pr create title is linted" "$repo" 'gh pr create --draft --title Bad.' "PR title" +CLAUDE_PROJECT_DIR=$root expect_allow "gh pr new --draft with a good title" "$repo" 'gh pr new --draft --title "fix: add the thing"' +expect_block "approve through the REST API" "$repo" 'gh api repos/o/r/pulls/12/reviews -f event=APPROVE' "Only humans approve" +expect_block "request changes through the REST API" "$repo" "gh api -X POST repos/o/r/pulls/12/reviews -f event=REQUEST_CHANGES -f body='see inline'" "instead of --request-changes" +expect_block "submit a pending review as an approval" "$repo" 'gh api repos/{owner}/{repo}/pulls/12/reviews/34/events -f event=APPROVE' "Only humans approve" +expect_block "open a PR through the REST API without draft" "$repo" 'gh api repos/o/r/pulls -f head=feat -f base=main -f title="fix: x"' "must be drafts" +CLAUDE_PROJECT_DIR=$root expect_block "a draft opened through the REST API gets the title lint" "$repo" 'gh api repos/o/r/pulls -f head=feat -f base=main -F draft=true -f title="Bad title."' "PR title" +CLAUDE_PROJECT_DIR=$root expect_block "a retitle through the REST API gets the title lint" "$repo" 'gh api --method=PATCH repos/o/r/pulls/12 -f "title=Bad title."' "PR title" +CLAUDE_PROJECT_DIR=$root expect_allow "a draft opened through the REST API with a good title" "$repo" 'gh api repos/o/r/pulls -f head=feat -f base=main -F draft=true -f title="fix: add the thing"' +expect_block "ready for review through GraphQL" "$repo" "gh api graphql -f query='mutation { markPullRequestReadyForReview(input: {pullRequestId: \"PR_x\"}) { clientMutationId } }'" "ready-for-review" +expect_block "request reviewers with reviewers[]= and no -X" "$repo" "gh api repos/o/r/pulls/12/requested_reviewers -f 'reviewers[]=someone'" "Reviewer-write" +expect_block "remove a requested reviewer" "$repo" "gh api -X DELETE repos/o/r/pulls/12/requested_reviewers -f 'reviewers[]=someone'" "Reviewer-write" +expect_block "a quoted requested_reviewers path" "$repo" 'gh api -XPOST "repos/o/r/pulls/12/requested_reviewers" -f reviewers[]=someone' "Reviewer-write" +expect_block "request reviewers through GraphQL" "$repo" "gh api graphql -f query='mutation { requestReviews(input: {pullRequestId: \"PR_x\", userIds: [\"U_x\"]}) { clientMutationId } }'" "Reviewer-write" +expect_block "approve through GraphQL" "$repo" "gh api graphql -f query='mutation { addPullRequestReview(input: {pullRequestId: \"PR_x\", event: APPROVE}) { clientMutationId } }'" "Only humans approve" +expect_block "open a non-draft PR through GraphQL" "$repo" "gh api graphql -f query='mutation { createPullRequest(input: {repositoryId: \"R_x\", baseRefName: \"main\", headRefName: \"feat\", title: \"fix: x\"}) { clientMutationId } }'" "must be drafts" +expect_block "merge through the REST API" "$repo" 'gh api -X PUT repos/o/r/pulls/12/merge -f merge_method=squash' "don't merge" +expect_block "merge through GraphQL" "$repo" "gh api graphql -f query='mutation { mergePullRequest(input: {pullRequestId: \"PR_x\"}) { clientMutationId } }'" "don't merge" +expect_block "auto-merge through GraphQL" "$repo" "gh api graphql -f query='mutation { enablePullRequestAutoMerge(input: {pullRequestId: \"PR_x\"}) { clientMutationId } }'" "don't merge" +# gh pr merge itself is left to the deny rule in .claude/settings.json: a +# person merges with it from shell mode, which this hook may see. +expect_allow "gh pr merge is left to the settings.json deny rule" "$repo" 'gh pr merge 12 --squash' + +# Look-alikes that read, or don't touch a PR, go through. +expect_allow "read a PR through the REST API" "$repo" 'gh api repos/o/r/pulls/12' +expect_allow "list PRs" "$repo" "gh api 'repos/o/r/pulls?state=open'" +expect_allow "list PRs with -X GET and fields" "$repo" 'gh api -X GET repos/o/r/pulls -f state=open' +expect_allow "read a PR's reviews" "$repo" 'gh api repos/o/r/pulls/12/reviews' +expect_allow "read its requested reviewers" "$repo" 'gh api repos/o/r/pulls/12/requested_reviewers' +expect_allow "check whether it merged" "$repo" 'gh api repos/o/r/pulls/12/merge' +expect_allow "a COMMENT review through the REST API" "$repo" "gh api -X POST repos/o/r/pulls/12/reviews -f event=COMMENT -f body='looks fine'" +expect_allow "a GraphQL query that reads reviews" "$repo" "gh api graphql -f query='query { repository(owner: \"o\", name: \"r\") { pullRequest(number: 12) { reviews(last: 5) { nodes { state } } } } }'" +expect_allow "a draft PR opened through GraphQL" "$repo" "gh api graphql -f query='mutation { createPullRequest(input: {repositoryId: \"R_x\", baseRefName: \"main\", headRefName: \"feat\", title: \"fix: x\", draft: true}) { clientMutationId } }'" +expect_allow "an issue comment that mentions approve and merge" "$repo" "gh api -X POST repos/o/r/issues/12/comments -f body='approve it, then merge'" +expect_allow "gh pr view" "$repo" 'gh pr view 12' + +# A gh command only mentioned in a heredoc body, a comment or a quoted string +# isn't run; one a shell runs still is. +expect_allow "a gh pr ready in a comment inside a python heredoc" "$repo" "python3 - <<'EOF' +# gh pr ready is humans-only. +print('ok') +EOF" +expect_allow "gh pr merge and an API merge in a cat heredoc" "$repo" "cat <<'EOF' >notes.txt +gh pr merge 12 --squash +gh api -X PUT repos/o/r/pulls/12/merge +EOF" +expect_allow "a gh pr ready in a shell comment" "$repo" 'ls # then gh pr ready 12' +expect_allow "a gh pr ready in a quoted string" "$repo" 'echo "run gh pr ready 12 when done"' +expect_allow "a PR body heredoc that mentions gh pr ready" "$repo" "gh pr create --draft --title \"fix: x\" --body \"\$(cat <<'EOF' +A person runs gh pr ready 12 later. +EOF +)\"" +expect_block "a gh pr ready run on its own line" "$repo" "cd . +gh pr ready 12" "ready-for-review" +expect_block "a gh pr ready behind a wrapper" "$repo" 'timeout 60 gh pr ready 12' "ready-for-review" +# shellcheck disable=SC2016 # a literal $(…), for the gate to read +expect_block "a gh pr ready in a \$(…)" "$repo" 'echo "$(gh pr ready 12)"' "ready-for-review" +expect_block "a gh pr ready handed to bash -c" "$repo" "bash -c 'gh pr ready 12'" "ready-for-review" +expect_block "a gh pr ready in a heredoc read by bash" "$repo" "bash <<'EOF' +gh pr ready 12 +EOF" "ready-for-review" +expect_block "a gh pr ready in a heredoc piped into sh" "$repo" "cat <<'EOF' | sh +gh pr ready 12 +EOF" "ready-for-review" + +# gh api handed to a shell is judged like any other gh api call. +expect_block "an API merge handed to bash -c" "$repo" "bash -c 'gh api -X PUT repos/o/r/pulls/12/merge'" "don't merge" +expect_block "an API approval handed to sh -c" "$repo" 'sh -c "gh api repos/o/r/pulls/12/reviews -f event=APPROVE"' "Only humans approve" +expect_block "GraphQL ready handed to bash -lc" "$repo" "bash -lc \"gh api graphql -f query='mutation { markPullRequestReadyForReview(input: {pullRequestId: \\\"PR_x\\\"}) { clientMutationId } }'\"" "ready-for-review" +expect_block "an API merge handed to eval" "$repo" "eval 'gh api -X PUT repos/o/r/pulls/12/merge'" "don't merge" +expect_allow "a read handed to bash -c" "$repo" "bash -c 'gh api repos/o/r/pulls/12'" + +# A boolean flag set with = counts as set when its value is true, and not when false. +expect_allow "gh pr create --draft=true" "$repo" 'gh pr create --draft=true --title "fix: x"' +expect_block "gh pr create --draft=false" "$repo" 'gh pr create --draft=false --title "fix: x"' "must use --draft" +expect_block "gh pr review --approve=true" "$repo" 'gh pr review 12 --approve=true' "Only humans approve" +expect_allow "gh pr review --approve=false --comment" "$repo" 'gh pr review 12 --approve=false --comment -b "noted"' +expect_block "gh pr review --request-changes=true" "$repo" 'gh pr review 12 --request-changes=true -b "see inline"' "instead of --request-changes" +expect_block "gh pr edit --add-reviewer=someone" "$repo" 'gh pr edit 12 --add-reviewer=someone' "Adding/removing reviewers" + +# gh pr create / new can request reviewers or assign people at creation; on gh pr review -r / -a stay --request-changes / --approve. +for c in create new; do + expect_block "gh pr $c --reviewer x" "$repo" "gh pr $c --draft --title \"feat: x\" --reviewer x" "Adding/removing reviewers" + expect_block "gh pr $c --reviewer=x" "$repo" "gh pr $c --draft --title \"feat: x\" --reviewer=x" "Adding/removing reviewers" + expect_block "gh pr $c -r x" "$repo" "gh pr $c --draft --title \"feat: x\" -r x" "Adding/removing reviewers" + expect_block "gh pr $c -rx (glued)" "$repo" "gh pr $c --draft --title \"feat: x\" -rx" "Adding/removing reviewers" + expect_block "gh pr $c --assignee x" "$repo" "gh pr $c --draft --title \"feat: x\" --assignee x" "Adding/removing reviewers" + expect_block "gh pr $c --assignee=x" "$repo" "gh pr $c --draft --title \"feat: x\" --assignee=x" "Adding/removing reviewers" + expect_block "gh pr $c -a x" "$repo" "gh pr $c --draft --title \"feat: x\" -a x" "Adding/removing reviewers" + expect_block "gh pr $c -a=x (glued)" "$repo" "gh pr $c --draft --title \"feat: x\" -a=x" "Adding/removing reviewers" +done +expect_allow "a plain gh pr create --draft" "$repo" 'gh pr create --draft --title "feat: x"' +expect_block "gh pr review -a is still --approve" "$repo" 'gh pr review 12 -a' "Only humans approve" +expect_block "gh pr review -r is still --request-changes" "$repo" 'gh pr review 12 -r -b "see inline"' "instead of --request-changes" + +# Each gh api call is judged by its own endpoint and fields, not by text in a +# field's value or in another command on the line. +expect_allow "a review-thread reply that quotes a merge endpoint" "$repo" "gh api repos/o/r/pulls/12/comments/34/replies -f body='Merging is PUT repos/o/r/pulls/12/merge and only a person runs it.'" +expect_allow "an issue comment that names requested_reviewers" "$repo" "gh api -X POST repos/o/r/issues/12/comments -f body='requested_reviewers is set by the ruleset'" +expect_allow "a comment body with event=APPROVE in it" "$repo" "gh api -X POST repos/o/r/issues/12/comments -f body='a review sends event=APPROVE; we do not'" +expect_allow "a GraphQL comment that mentions a mutation name" "$repo" "gh api graphql -f query='mutation(\$b: String!) { addComment(input: {subjectId: \"I_x\", body: \$b}) { clientMutationId } }' -f b='markPullRequestReadyForReview is for people'" +expect_allow "a reviewer read, then a comment, on one line" "$repo" 'gh api repos/o/r/pulls/12/requested_reviewers && gh api -X POST repos/o/r/issues/12/comments -f body=ping' +expect_block "a comment, then a reviewer write, on one line" "$repo" "gh api -X POST repos/o/r/issues/12/comments -f body=ping && gh api repos/o/r/pulls/12/requested_reviewers -f 'reviewers[]=someone'" "Reviewer-write" +expect_allow "a draft opened through the REST API whose body says draft=false" "$repo" "gh api repos/o/r/pulls -f head=feat -f base=main -F draft=true -f body='not draft=false'" +expect_block "a non-draft whose body says draft=true" "$repo" "gh api repos/o/r/pulls -f head=feat -f base=main -f body='draft=true'" "must be drafts" +expect_block "an API merge through a full URL" "$repo" 'gh api --method PUT https://api.github.com/repos/o/r/pulls/12/merge' "don't merge" +expect_block "an API merge with fields set by -F=" "$repo" 'gh api repos/o/r/pulls/12/merge --field=merge_method=squash' "don't merge" + +# The gate follows 64 pieces of nested code; a gh command beyond them is refused. +# shellcheck disable=SC2016 # literal $(true)s, for the gate to read +pad62=$(printf ' $(true)%.0s' $(seq 62)) pad63=$(printf ' $(true)%.0s' $(seq 63)) +expect_block "a gh pr ready in the 63rd substitution is judged" "$repo" "echo$pad62 \$(gh pr ready 12)" "ready-for-review" +expect_block "a gh pr ready past the 64 the gate follows is refused" "$repo" "echo$pad63 \$(gh pr ready 12)" "nests more code" +expect_allow "a gh read beside 70 substitutions with no gh in them" "$repo" "gh pr view 12 && echo$pad63 \$(true) \$(true) \$(true) \$(true) \$(true) \$(true) \$(true)" + +# gh resolves -R/--repo before the command. +expect_block "gh -R o/r pr ready" "$repo" 'gh -R o/r pr ready 12' "ready-for-review" +expect_block "gh --repo o/r pr review --approve" "$repo" 'gh --repo o/r pr review 12 --approve' "Only humans approve" +expect_block "gh --repo=o/r pr create without --draft" "$repo" 'gh --repo=o/r pr create --title "fix: x"' "must use --draft" +expect_block "gh -Ro/r pr edit --add-reviewer" "$repo" 'gh -Ro/r pr edit 12 --add-reviewer someone' "Adding/removing reviewers" +expect_block "gh -R o/r api merge" "$repo" 'gh -R o/r api -X PUT repos/o/r/pulls/12/merge' "don't merge" +expect_allow "gh -R o/r pr view" "$repo" 'gh -R o/r pr view 12' + +# A GraphQL query computed by a heredoc inside $(…) is read from that heredoc. +expect_block "GraphQL ready in a heredoc query" "$repo" "gh api graphql -f query=\"\$(cat <<'EOF' +mutation { markPullRequestReadyForReview(input: {pullRequestId: \"PR_x\"}) { clientMutationId } } +EOF +)\"" "ready-for-review" +expect_allow "a GraphQL read in a heredoc query" "$repo" "gh api graphql -f query=\"\$(cat <<'EOF' +query { viewer { login } } +EOF +)\"" + +# A shell given options before -c still runs the code after them; the push +# check and the gh check agree on these forms. +expect_block "bash -e -c with a gh pr ready" "$repo" "bash -e -c 'gh pr ready 12'" "ready-for-review" +expect_block "bash -euo pipefail -c with a gh pr ready" "$repo" "bash -euo pipefail -c 'gh pr ready 12'" "ready-for-review" +expect_block "bash -o pipefail -c with a gh pr ready" "$repo" "bash -o pipefail -c 'gh pr ready 12'" "ready-for-review" +expect_block "bash +o posix -c with a gh pr ready" "$repo" "bash +o posix -c 'gh pr ready 12'" "ready-for-review" +expect_block "bash --noprofile --norc -c with an API merge" "$repo" "bash --noprofile --norc -c 'gh api -X PUT repos/o/r/pulls/12/merge'" "don't merge" +expect_block "bash -e -c with a push" "$repo" "bash -e -c 'git -C ../wt-b push'" "can't follow" +expect_block "bash -euo pipefail -c with a push" "$repo" "bash -euo pipefail -c 'git -C ../wt-b push'" "can't follow" +expect_block "bash --noprofile --norc -c with a push" "$repo" "bash --noprofile --norc -c 'git -C ../wt-b push'" "can't follow" +expect_allow "bash --norc running a script, not -c" "$repo" 'bash --norc scripts/pre-push-reviewers.sh' +expect_allow "bash -o pipefail -c with a read" "$repo" "bash -o pipefail -c 'gh pr view 12'" + +# The code inside a $(…) is judged as written, its quotes kept. +expect_block "a GraphQL ready mutation captured by \$(…)" "$repo" "resp=\$(gh api graphql -f query='mutation { markPullRequestReadyForReview(input: {pullRequestId: \"PR_x\"}) { clientMutationId } }')" "ready-for-review" +# shellcheck disable=SC2016 # a literal $(…), for the gate to read +CLAUDE_PROJECT_DIR=$root expect_allow "a draft PR's URL captured by \$(…), two-word title" "$repo" 'url=$(gh pr create --draft --title "fix: add the thing" --body-file b.md)' +# shellcheck disable=SC2016 +CLAUDE_PROJECT_DIR=$root expect_allow "a draft PR's URL captured by \$(…), scoped title" "$repo" 'url=$(gh pr create --draft --title "feat(gate): add the thing" --body-file b.md)' +# shellcheck disable=SC2016 +CLAUDE_PROJECT_DIR=$root expect_block "a bad title inside \$(…) is still linted" "$repo" 'url=$(gh pr create --draft --title "Bad title." --body-file b.md)' "PR title" +expect_allow "a comment body with an apostrophe inside \$(…)" "$repo" "out=\$(gh pr comment 12 --body \"it's fixed\")" +expect_allow "a gh pr ready only grepped for inside \$(…)" "$repo" "n=\$(grep -c 'gh pr ready' AGENTS.md)" +expect_block "a gh pr ready in a heredoc read by bash inside \$(…)" "$repo" "out=\$(bash <<'EOF' +gh pr ready 12 +EOF +)" "ready-for-review" + if [ "$fails" -gt 0 ]; then printf '\n%d case(s) failed\n' "$fails" >&2 exit 1 diff --git a/.claude/settings.json b/.claude/settings.json index 95c28c02..604c99a6 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -36,7 +36,7 @@ "hooks": [ { "type": "command", - "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/agent-bash-gate.sh", + "command": "h=\"$CLAUDE_PROJECT_DIR/.claude/hooks/agent-bash-gate.sh\"; [ -f \"$h\" ] && [ -x \"$h\" ] && exec \"$h\"; c=$(jq -r '.tool_input.command // empty' 2>/dev/null) || { c='git push'; cat >/dev/null; }; m() { printf '%s\\n' \"$c\" | grep -Eq '(^|[^[:alnum:]_.-])('\"$1\"')([^[:alnum:]_.-]|$)'; }; if { m git && m push; } || { m gh && { { m pr && m 'create|new|edit|ready|review|merge'; } || { m api && m 'pulls|graphql'; }; }; }; then echo \"🛑 Claude PR discipline gate: $h is missing or not executable, so this push or PR change can't be checked and is blocked (without jq, every command is). If this session's checkout was removed, restart Claude Code from a live checkout.\" >&2; exit 2; fi", "timeout": 30 } ] @@ -48,11 +48,11 @@ "hooks": [ { "type": "command", - "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/gofumpt-on-save.sh" + "command": "h=\"$CLAUDE_PROJECT_DIR/.claude/hooks/gofumpt-on-save.sh\"; [ -f \"$h\" ] && [ -x \"$h\" ] && exec \"$h\"; cat >/dev/null; echo \"⚠️ $h is missing or not executable, so the file was not auto-formatted (the edit itself went through). If this session's checkout was removed, restart Claude Code from a live checkout.\" >&2; exit 2" }, { "type": "command", - "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/markdown-on-save.sh" + "command": "h=\"$CLAUDE_PROJECT_DIR/.claude/hooks/markdown-on-save.sh\"; [ -f \"$h\" ] && [ -x \"$h\" ] && exec \"$h\"; cat >/dev/null; echo \"⚠️ $h is missing or not executable, so the file was not auto-formatted (the edit itself went through). If this session's checkout was removed, restart Claude Code from a live checkout.\" >&2; exit 2" } ] } @@ -62,7 +62,7 @@ "hooks": [ { "type": "command", - "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/review-marker.sh" + "command": "h=\"$CLAUDE_PROJECT_DIR/.claude/hooks/review-marker.sh\"; [ -f \"$h\" ] && [ -x \"$h\" ] && exec \"$h\"; cat >/dev/null; echo \"⚠️ $h is missing or not executable, so a reviewer that finishes now writes no marker and a push will find none. If this session's checkout was removed, restart Claude Code from a live checkout.\" >&2; exit 1" } ] } diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 71736be3..07d55cfe 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,13 +1,38 @@ #!/usr/bin/env bash # Runs `make verify` (or skips if `scripts/ci-marker.sh` says it already passed # for this tree). Bypass: `git commit --no-verify`. +# +# An absolute core.hooksPath runs this hook in every worktree of the +# repository, including ones whose branch holds no code (an orphan branch that +# keeps a tool's state, say). A worktree with no Makefile has nothing to verify. set -uo pipefail cd "$(git rev-parse --show-toplevel)" || exit 1 -if scripts/ci-marker.sh has-verify-marker 2>/dev/null; then - echo "✔ pre-commit: verify already passed for this tree (cached)" >&2 +if [ ! -f Makefile ]; then + echo "pre-commit: no Makefile in $PWD, so nothing to verify." >&2 exit 0 fi +# has-verify-marker exits 0 when the marker is there and 1 when it isn't; any +# other failure, or a missing script, is reported and falls through to the full +# `make verify`. +marker_script=scripts/ci-marker.sh +if [ ! -x "$marker_script" ]; then + echo "⚠️ pre-commit: $marker_script is missing or not executable, so the cached verify result can't be checked; running 'make verify'." >&2 +else + err=$("$marker_script" has-verify-marker 2>&1 >/dev/null) + case $? in + 0) + echo "✔ pre-commit: verify already passed for this tree (cached)" >&2 + exit 0 + ;; + 1) ;; + *) + echo "⚠️ pre-commit: $marker_script has-verify-marker failed, so the cached verify result can't be checked; running 'make verify'." >&2 + [ -n "$err" ] && printf '%s\n' "$err" | sed 's/^/ /' >&2 + ;; + esac +fi + make verify || { echo "🛑 pre-commit: 'make verify' failed." >&2; exit 1; } diff --git a/.githooks/pre-push b/.githooks/pre-push index 6050848b..85eea4a7 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -4,19 +4,24 @@ # allowlist CI uses (scripts/classify-paths.sh), so local and CI agree on # what a change "is": # - any code change → full `make ci` must have passed (tree-keyed marker -# tmp/ci-passed-tree-<tree>), exactly as before; +# tmp/ci-passed-tree-<tree>), exactly as before, here or on another +# machine via `make ci-remote`; # - docs/prose-only → `make verify` is enough (CI skips the Go/SDK suites # for these too), so only the verify marker is required — no spinning # up the local Docker suites for a typo fix. pre-commit already runs # `make verify`, so this is usually a zero-extra-work push. # Fail-closed: if the change set can't be classified (no resolvable base, # classifier error), fall back to requiring the full `make ci` marker. +# A pushed commit with no Makefile holds no code to validate (an orphan branch +# that keeps a tool's state, say), so it passes; one with a Makefile, pushed +# from a worktree that lacks scripts/ci-marker.sh, is blocked. # Bypass: `git push --no-verify` (humans only). set -uo pipefail cd "$(git rev-parse --show-toplevel)" || exit 1 zero=0000000000000000000000000000000000000000 +marker_script=scripts/ci-marker.sh # Net changed files for a pushed ref (base..tip), one per line. New branch # (no remote tip) → diff against the merge-base with origin/main. Returns @@ -33,19 +38,32 @@ changed_files() { block_ci() { # <sha> — require the full make-ci marker local sha="$1" marker tree - marker=$(scripts/ci-marker.sh path-for-commit "$sha" 2>/dev/null) || { + marker=$("$marker_script" path-for-commit "$sha" 2>/dev/null) || { echo "🛑 pre-push: can't resolve tree for ${sha:0:8}." >&2; exit 1 } if [ ! -f "$marker" ]; then tree="${marker#tmp/ci-passed-tree-}" - echo "🛑 pre-push: 'make ci' has not been run for ${sha:0:8} (tree ${tree:0:8}). Run 'make ci' or 'git push --no-verify'." >&2 + echo "🛑 pre-push: 'make ci' has not been run for ${sha:0:8} (tree ${tree:0:8}). Run 'make ci' (or 'make ci-remote HOST=<host>') or 'git push --no-verify'." >&2 exit 1 fi + # `make ci-remote` leaves a provenance file beside its marker. + if [ -f "$marker.provenance" ]; then + prov() { sed -n "s/^$1=//p" "$marker.provenance"; } + echo "✔ pre-push: make ci passed on $(prov host) ($(prov os)/$(prov arch)) for ${sha:0:8}." >&2 + fi } while read -r _ local_sha _ remote_sha; do [ -z "${local_sha:-}" ] && continue [ "$local_sha" = "$zero" ] && continue # branch deletion — nothing to validate + if ! git rev-parse --verify --quiet "$local_sha:Makefile" >/dev/null; then + echo "pre-push: ${local_sha:0:8} has no Makefile, so no code to validate." >&2 + continue + fi + if [ ! -x "$marker_script" ]; then + echo "🛑 pre-push: $marker_script is missing or not executable in $PWD, so the validation marker for ${local_sha:0:8} can't be checked. Push from a worktree that has it." >&2 + exit 1 + fi # Classify the push, failing closed to a code change on any hiccup. code=true @@ -57,7 +75,7 @@ while read -r _ local_sha _ remote_sha; do fi if [ "$code" = false ]; then - marker=$(scripts/ci-marker.sh verify-path-for-commit "$local_sha" 2>/dev/null) || { + marker=$("$marker_script" verify-path-for-commit "$local_sha" 2>/dev/null) || { echo "🛑 pre-push: can't resolve tree for ${local_sha:0:8}." >&2; exit 1 } if [ ! -f "$marker" ]; then diff --git a/AGENTS.md b/AGENTS.md index 1f8079d1..7a23d51d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ This file provides context for AI coding agents (Copilot, Cursor, Cody, Aider, e The non-negotiables, ordered by how often agents miss them. Each links to its detail section — read that before acting. These override convenience: if a rule blocks you, satisfy it; don't work around it. -1. **Validate locally before every push** — run `make ci` the documented way ([§Running `make ci`](#running-make-ci-for-agents)). Don't use CI as your first feedback loop. +1. **Validate locally before every push** — run `make ci` the documented way, here or on a configured host with `make ci-remote` ([§Running `make ci`](#running-make-ci-for-agents)). Don't use CI as your first feedback loop. 2. **A PR-branch push needs every pre-push reviewer satisfied** — run **`/prepush`**, which discovers the reviewers from `scripts/pre-push-reviewers.sh`, runs the ones the change needs in parallel (fresh context), skips any with nothing to do *on the record*, and loops until each it ran returns `ship_it`. Every reviewer needs a marker for the commit being pushed — earned by a `ship_it` or a logged skip; the set is the single source of truth and grows over time (code, docs, security, …), so never hardcode it ([§Pre-push self-review](#pre-push-self-review-is-mandatory-on-pr-branches)). 3. **Every code change updates its docs + `CHANGELOG.md` in the same PR** — a code change without its doc update is incomplete ([§Documentation Sync](#documentation-sync)). 4. **Address and resolve every review finding** — substantive reply, fix it or track it in an issue, @-mention the bot, then resolve; never silently drop one ([§Review Response](#review-response)). @@ -169,12 +169,14 @@ NO_COLOR=1 make ci > tmp/ci.log 2>&1 On success `make ci` writes the tree-keyed `tmp/ci-passed-tree-<TREE>` marker (see §Enforced via git hooks for the tree-keying and the commit-after-green rule; `tmp/` is gitignored, so the marker never enters the tree). That's one of the markers a PR-branch push requires — the rest are written by the mandatory review subagents, one per reviewer in `scripts/pre-push-reviewers.sh` (see §Agent PR Discipline → Pre-push self-review). End to end: `make ci` green → commit → run every pre-push reviewer in parallel (fresh context) via `/prepush` → loop until each reaches `ship_it` → push. Re-run `make ci` only if a finding makes you edit a tracked file. +**On another machine.** When a host is configured (an ssh destination, or `docker:<container>`), you may satisfy the pre-push gate with `make ci-remote HOST=<host>` instead. It runs `make ci` for `HEAD` there and writes the same `tmp/ci-passed-tree-<TREE>` marker here only if that run passed for exactly this tree, so the gate's meaning is unchanged. Only `HEAD` travels, so commit first (it refuses a dirty worktree), and don't commit while it runs. Run it in the background exactly like `make ci` (`NO_COLOR=1 make ci-remote HOST=<host> > tmp/ci.log 2>&1`). Host requirements: `docs/src/content/docs/development.md` § Running `make ci` on another machine. + ### Enforced via git hooks -`make tools` installs team-wide git hooks via `git config core.hooksPath .githooks`. They apply to humans and Claude Code alike: +`make tools` installs team-wide git hooks via `git config core.hooksPath .githooks`. They apply to humans and Claude Code alike. Git resolves that relative path in each worktree, so each worktree runs its own branch's hooks, and a worktree whose branch has no `.githooks/` runs none; an absolute `core.hooksPath` runs one checkout's hooks in every worktree instead. The hooks behave correctly under either: -- **`.githooks/pre-commit`** runs `make verify` (~30s) on every commit; blocks on failure. Skipped if `make ci` or `make verify` already ran for the current tree state (cached marker). See §Documentation Sync / §SDK Sync for what to update by hand. -- **`.githooks/pre-push`** scales the bar to what the push changes, classified by the same allowlist CI uses (`scripts/classify-paths.sh`): a code change requires the full `make ci` marker (`tmp/ci-passed-tree-<TREE-sha>`), while a docs/prose-only push requires only the `make verify` marker (`tmp/verify-passed-tree-<TREE-sha>`) — CI skips the Go/SDK suites for those too, so there's no reason to run them locally (and pre-commit's `make verify` usually already wrote it). Fail-closed: an unclassifiable push (no resolvable base, classifier error) falls back to requiring `make ci`. Both markers are tree-keyed (not commit-keyed) so `make {ci,verify} → commit → push` works without a re-run when the tree is unchanged; editing the tree (or staging a different subset than CI saw) requires a re-run. The marker write is skipped entirely when `$CI` is set (CI runners don't push). +- **`.githooks/pre-commit`** runs `make verify` (~30s) on every commit; blocks on failure. Skipped if `make ci` or `make verify` already ran for the current tree state (cached marker). A worktree with no `Makefile` (an orphan branch that keeps a tool's state, say) has nothing to verify and commits. A missing or failing `scripts/ci-marker.sh` is reported, not hidden, and `make verify` runs. See §Documentation Sync / §SDK Sync for what to update by hand. +- **`.githooks/pre-push`** scales the bar to what the push changes, classified by the same allowlist CI uses (`scripts/classify-paths.sh`): a code change requires the full `make ci` marker (`tmp/ci-passed-tree-<TREE-sha>`), while a docs/prose-only push requires only the `make verify` marker (`tmp/verify-passed-tree-<TREE-sha>`) — CI skips the Go/SDK suites for those too, so there's no reason to run them locally (and pre-commit's `make verify` usually already wrote it). Fail-closed: an unclassifiable push (no resolvable base, classifier error) falls back to requiring `make ci`. A pushed commit with no `Makefile` has no code to validate and goes through; one with a `Makefile`, pushed from a worktree that lacks `scripts/ci-marker.sh`, is blocked. Both markers are tree-keyed (not commit-keyed) so `make {ci,verify} → commit → push` works without a re-run when the tree is unchanged; editing the tree (or staging a different subset than CI saw) requires a re-run. The marker write is skipped entirely when `$CI` is set (CI runners don't push). Bypass with `git commit --no-verify` / `git push --no-verify` only when explicitly intentional (WIP / draft pushes where you accept the consequences). Don't disable the hooks globally; that defeats the gate. @@ -238,15 +240,19 @@ See also: `.claude/skills/pr-sync-with-main/SKILL.md` for the same workflow in C Agents follow the same universal git hooks as humans (pre-commit + pre-push in `.githooks/`). On top of that, four PR-workflow rules have no human analog and are checked by `.claude/hooks/agent-bash-gate.sh`. The gate is a *guard rail against accidents*, not adversarial enforcement — an agent that wants to bypass can edit the gate itself. The rules below are policy; follow them. +**If the hook scripts are missing, the hooks fail closed, not open.** `.claude/settings.json` runs each hook through a short inline guard. When `$CLAUDE_PROJECT_DIR/.claude/hooks/<script>.sh` is there and executable, the guard `exec`s it, unchanged. When it isn't, for example because the session's checkout was removed, the bare path would exit 127, which Claude Code [treats as a non-blocking error](https://code.claude.com/docs/en/hooks#other-exit-codes), so the guard handles it itself. The PreToolUse guard blocks (exit 2) any command that contains the words `git` and `push`; `gh` and `pr` with one of `create`, `new`, `edit`, `ready`, `review` or `merge`; or `gh` and `api` with `pulls` or `graphql`, which covers every call to the pull-request or GraphQL API, reads included. It also blocks every command when there is no `jq` to read it with. Other commands go through, so the session can still work. The SubagentStop guard reports a missing `review-marker.sh` with exit 1, because exit 2 there [keeps the subagent running](https://code.claude.com/docs/en/hooks#exit-code-2-behavior-per-event); the push gate then finds no marker. The PostToolUse guards report a missing formatter with exit 2, which shows Claude the message after the edit. Each message says what to do: restart Claude Code from a live checkout. The match is crude on purpose, like the gate it stands in for. `make test-hook-commands` runs every command from the real `settings.json` with its script present, missing and not executable. The per-event table and the doc citations are in [the Claude Code guide](docs/src/content/docs/claude-code.md#when-a-hook-script-is-missing). + ### Drafts only Agents must create PRs with `gh pr create --draft`. Only humans transition draft → ready-for-review (`gh pr ready` is blocked for agents). Only humans approve or request changes (`gh pr review --approve` / `--request-changes` are blocked). -**PR title format.** The title becomes the squash-merge subject on `main` and is gated by the required `CI` check's `PR title` job — a bad title blocks merge, so don't discover it from CI. It must be Conventional Commits — `<type>(optional-scope)(optional-!): <subject>` — **≤ 72 chars**, subject **lowercase-first** with **no trailing period**. Types: `feat fix docs refactor test chore ci deps build perf revert style`. Validate before creating: `scripts/lint-pr-title.sh "<title>"` (exit 0 = valid; it prints the reason on failure). `.claude/hooks/agent-bash-gate.sh` runs the same check on `gh pr create` / `gh pr edit --title`, so a malformed title is caught locally before the PR exists. The rule has a single source of truth — `scripts/lint-pr-title.sh` — called by this local gate, by the `PR title` job in `ci.yml` (from a trusted `main` checkout), and by `housekeeping.yml`'s advisory sticky-comment mirror, so local and CI never drift. Fixing a title after the fact needs no new push: editing it triggers housekeeping, which re-runs the failed `PR title` job (the job re-reads the title from the API). +The gate checks these rules in their other forms too. `gh pr new` is an alias of `gh pr create`. Through the REST API, `gh api …/pulls` that opens a PR must send `-F draft=true`, and `…/pulls/<N>/reviews` with `event=APPROVE` or `event=REQUEST_CHANGES` is blocked. Through GraphQL, `createPullRequest` without `draft: true`, `markPullRequestReadyForReview`, and `addPullRequestReview` or `submitPullRequestReview` with `APPROVE` or `REQUEST_CHANGES` are blocked. It also blocks a merge through the API (`PUT …/pulls/<N>/merge`, `mergePullRequest`, `enablePullRequestAutoMerge`). `gh pr merge` itself is denied for Claude's commands in `.claude/settings.json` and left out of the gate, because a person merges with it from shell mode (`!`), which the Claude Code docs don't say whether this hook sees. The gate reads the commands a line runs, like the push check: a gh command only mentioned in a quoted string, a heredoc body or a comment doesn't count, unless a shell runs it (a `$(…)`, `bash -c '…'`, `eval`, or a heredoc a shell reads). Each `gh api` call is judged by its own endpoint (its first argument that isn't a flag) and its own fields, so a comment body that names an endpoint, or another call on the line, doesn't count. A line it can't parse is refused when it mentions a `gh pr` or `gh api` command, and so is such a command left beyond the 64 pieces of nested code the gate follows. `gh -R <repo> pr …` (or `--repo`) counts as `gh pr …`, a shell given options before `-c` (`bash -euo pipefail -c '…'`) still hands its code over, and a GraphQL query computed by a heredoc inside `$(…)` is read from that heredoc. It reads the command line only, so fields sent through `--input` or `-F field=@file` go unseen. + +**PR title format.** The title becomes the squash-merge subject on `main` and is gated by the required `CI` check's `PR title` job — a bad title blocks merge, so don't discover it from CI. It must be Conventional Commits — `<type>(optional-scope)(optional-!): <subject>` — **≤ 72 chars**, subject **lowercase-first** with **no trailing period**. Types: `feat fix docs refactor test chore ci deps build perf revert style`. Validate before creating: `scripts/lint-pr-title.sh "<title>"` (exit 0 = valid; it prints the reason on failure). `.claude/hooks/agent-bash-gate.sh` runs the same check on `gh pr create` / `gh pr new` / `gh pr edit --title`, and on the `title=` field of a `gh api` call that opens or edits a PR, so a malformed title is caught locally before the PR exists. The rule has a single source of truth — `scripts/lint-pr-title.sh` — called by this local gate, by the `PR title` job in `ci.yml` (from a trusted `main` checkout), and by `housekeeping.yml`'s advisory sticky-comment mirror, so local and CI never drift. Fixing a title after the fact needs no new push: editing it triggers housekeeping, which re-runs the failed `PR title` job (the job re-reads the title from the API). ### Human reviewer assignment is humans-only -Adding/removing human reviewers (`gh pr edit --add-reviewer <login>`, `gh pr edit --add-assignee <login>`, or `POST /repos/.../pulls/<N>/requested_reviewers`) is blocked for agents. Reviewer assignment is handled natively by GitHub — the `required_reviewers` rule requests the `@Wave-RF/wavehouse-admins` team and the team's code-review assignment picks the member; humans handle anything else. +Adding/removing human reviewers (`gh pr edit --add-reviewer <login>`, `gh pr edit --add-assignee <login>`, `--reviewer` / `--assignee` (or `-r` / `-a`) on `gh pr create` / `gh pr new`, any `gh api` write to `/repos/.../pulls/<N>/requested_reviewers`, or the GraphQL `requestReviews` mutation) is blocked for agents. Reviewer assignment is handled natively by GitHub — the `required_reviewers` rule requests the `@Wave-RF/wavehouse-admins` team and the team's code-review assignment picks the member; humans handle anything else. ### Bot reviewer re-triggers go through PR comments @@ -286,7 +292,7 @@ That's all: the marker is `tmp/<name>-passed-<sha>` automatically, the push gate ### Don't bypass the gates - `--no-verify` on `git commit` / `git push` exists for human WIP / draft pushes. Agents should not use it. -- Markers are written by tooling, never by hand: `tmp/ci-passed-tree-*` by `make ci`; `tmp/<reviewer>-passed-*` (one per reviewer in `scripts/pre-push-reviewers.sh`) by the `review-marker.sh` SubagentStop hook on `ship_it`, **or** by `scripts/skip-pre-push-review.sh` for a deliberately-skipped reviewer (which logs the reason to `tmp/review-skips-<HEAD>.log`). Don't `touch` / `Write` / `Edit` a marker by hand — to skip a reviewer, use the skip command so the skip is recorded; if you're tempted to hand-write a review marker any other way, the marker is wrong-shaped for your situation. Run `make ci`, run or skip each reviewer, get the verdicts. +- Markers are written by tooling, never by hand: `tmp/ci-passed-tree-*` by `make ci` or `make ci-remote`; `tmp/<reviewer>-passed-*` (one per reviewer in `scripts/pre-push-reviewers.sh`) by the `review-marker.sh` SubagentStop hook on `ship_it`, **or** by `scripts/skip-pre-push-review.sh` for a deliberately-skipped reviewer (which logs the reason to `tmp/review-skips-<HEAD>.log`). Don't `touch` / `Write` / `Edit` a marker by hand — to skip a reviewer, use the skip command so the skip is recorded; if you're tempted to hand-write a review marker any other way, the marker is wrong-shaped for your situation. Run `make ci`, run or skip each reviewer, get the verdicts. These are policy, not mechanically enforced. Bash can write a file a dozen ways; an agent can edit `.claude/hooks/agent-bash-gate.sh` itself. Trust beats whack-a-mole regex. diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e325ff9..dc4b8f35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), - **Docs-site analytics for search, code copies, 404s, docs section, and live-demo connectivity** (`docs/src/components/DocsTracking.astro` (new), `docs/src/components/{PostHog,Footer,LiveDemo}.astro`): the site tracked its own CTAs but nothing a reader did on the way to one, so the questions that decide what to write next — what people search for and *don't* find, which snippets get copied, which dead links keep getting followed — had no data behind them. `docs_search` fires a second after the query settles rather than once per keystroke, carrying `query` and `result_count` read off Pagefind's own results message (the rendered list is capped at its page size, so counting the DOM would under-report); `result_count: 0` is the event worth having. `code_copied` (`page`, `language`) watches Expressive Code's copy buttons from the document rather than re-binding every code block on every navigation — the hero's install chip is not an EC block and keeps its own `hero_install_copied`. `docs_404` (`path`, `referrer`) turns broken inbound links into a list instead of a hunch. A `doc_section` property (the first path segment, `home` for `/`) puts every event in a docs area without each tracker carrying its own copy; it's stamped at capture time by a `before_send` hook in `posthog.init()` rather than `register()`, because a queued `register()` replays only after init has already captured the first hard-load `$pageview` — which would then carry the previous visit's persisted value — and `history_change` navigations update the URL before capture fires, so reading `location` in the hook is always current. `live_demo_connected` fires once per mount when the hero's SSE feed comes up rather than on its first row — named for what it measures (the demo backend answered), since a quiet minute on the repo is not a disengaged reader. The three site-wide trackers share one new `DocsTracking.astro` rendered from the footer (like `MermaidZoom` / `ScrollHints`) and delegate from `document`, since Pagefind, Expressive Code, and the 404 route all own their own markup — some of it created after page load. - **Dedupe retention per tenant and table, and a sweep that deletes expired ids** (`internal/settings/{settings,validate,store}.go` (+ tests), `internal/settings/seed/config.json`, `internal/dedupe/{embedded,sweep}.go` (+ tests), `internal/api/ingest.go` (+ tests), `internal/testutil/mocks.go`, `deployments/compose/settings/config.json`, `tests/e2e/fixtures/settings/config.json`, `config.yaml`, `docs/src/content/docs/{settings-directory.mdx,deployment,durability,architecture}.md`, `AGENTS.md`): [#220](https://github.com/Wave-RF/WaveHouse/issues/220). `config.json` gains an optional **`dedupe.retention`** key, overridable per table in `dedupe.tables.<table>.retention`: how long a committed id stays a duplicate, as a duration string (`"720h"`), or `"0"` to keep it forever, which is the seed value and the behaviour before this release. A `config.json` without the key keeps ids forever, and a table override without one inherits the tenant's, so an existing directory needs no change. A finite retention below `"2m"`, the ingest queue's duplicate window, is refused rather than raised to the minimum: every deduped record is published under an idempotency key derived from its id, so an id re-sent after a shorter retention would be claimed again and dropped by the queue as a copy while the client was told it was accepted. It is hot-reloadable and read per record like `id_field`; a change applies to ids committed after it, and a reload landing mid-window commits each record with the retention it was prepared under. The embedded Pebble store already treated an expired id as new; it now also deletes expired keys, and the version-0 keys the key-layout change left behind, in a background sweep that starts a minute after the instance opens and repeats hourly. It reads 1,024 keys per chunk without a lock, then re-reads the expired and version-0 ones under a lock `Commit` also takes and deletes, without fsync, those that still are, so an id committed again after the sweep read it is never deleted, and a `Commit` waits for at most one chunk's re-reads, never for the deleted keys a chunk steps over. New metric `wavehouse_dedupe_swept_keys_total{reason="expired"|"version_0"}`. `settings.Store.DedupeFor` now returns a `settings.Dedupe` struct rather than three values. - **`make build-ts` loads the built SDK entry points at the oldest supported Node** (`clients/ts/scripts/smoke-dist.mjs` (new), `clients/ts/{package.json,README.md}`, `clients/ts/scripts/fetch-node.sh` (new), `Makefile`, `biome.json`, `docs/src/content/docs/development.md`): closes [#498](https://github.com/Wave-RF/WaveHouse/issues/498). Nothing had loaded a built artifact, so a packaging break shipped green: an ESM-only dependency such as `eventsource-parser@4` would have made `require("@wavehouse/sdk")` throw `ERR_REQUIRE_ESM` on Node 22.0 to 22.11, inside the advertised `engines.node: ">=22"`. `make smoke-ts-dist`, which `build-ts` now runs after the build (so `test-e2e`, `build-all` and `make ci` all gate on it), reads the minimum from `engines.node` and runs `smoke-dist.mjs` under that exact Node downloaded once into `.bin/` by `clients/ts/scripts/fetch-node.sh` (verified against a pinned sha256 per platform, like shellcheck; `pnpm dlx` was rejected because pnpm 11 asks nodejs.org on every run and resolves a bare `node@x.y.z` to an unrelated npm package) and fails unless that is the Node it ran on, since `.nvmrc`'s floating `22` sits above the `require(esm)` line and cannot see the break. The script imports `@wavehouse/sdk` and `require()`s it (through the `exports` map, which must resolve to the declared files) and runs `dist/index.global.js` as a classic script in a fresh `vm` context, runs the codegen bin with `--help`, checks `main`, `module` and `jsdelivr` agree with `exports` and `unpkg`, takes the ESM build's export names as the reference surface, and fails if the CJS or IIFE surface differs in either direction. Entry point paths come from `package.json`, so none is hand-copied. Run by hand with `pnpm --filter @wavehouse/sdk run smoke:dist` on any Node. +- **`make ci-remote` runs `make ci` on another machine, and a pass there satisfies the pre-push gate** (`scripts/ci/remote-ci.sh` (new, + `remote-ci.test.sh`), `Makefile`, `.githooks/pre-push`, `scripts/ci-marker.sh`, `docs/src/content/docs/{development,claude-code}.md`, `AGENTS.md`, `CONTRIBUTING.md`): closes [#764](https://github.com/Wave-RF/WaveHouse/issues/764). A code push needs `tmp/ci-passed-tree-<tree>`, and only a `make ci` in the same checkout wrote it, so every push cost a full local run (the race detector, coverage, and a Docker daemon carrying every suite's containers) and only ever tested the contributor's own OS and architecture. `make ci-remote HOST=<host>` takes an ssh destination or `docker:<container>`, streams `HEAD` there as a `git bundle` (nothing is pushed), checks it out in a fresh directory, runs `make tools && make ci`, and streams the output back, the host's `uname -sm` first. Stopping it, by Ctrl-C or by a signal to the script alone, stops the run on the host as well: the host's stdin stays open only while the script runs, and a watcher stops the run's process group when it closes. It writes the markers a local `make ci` does only if the host exits 0, the tree the host reports having checked out is `HEAD^{tree}`, `make ci` left the host's checkout clean, and the local worktree is clean before and after the run and still on that tree; the host's report lines carry a per-run nonce, so `make ci`'s own output can't forge them. A provenance file beside the marker records the host, its OS and architecture, the start and end times and the run's own local copy of the log, and the pre-push hook prints `make ci passed on <host> (<os>/<arch>)` from it. What the marker means, and a local `make ci`, are unchanged. `make test-remote-ci`, a new `make verify` leaf, drives the script through stand-in `ssh` and `docker` commands against a scratch repository. ### Changed @@ -119,6 +120,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed - **`TestClaims_DifferingExtrasLeaveConfiguredUnitsAlone` no longer fails when a member's view lags the split** (`internal/ingest/claims_test.go`): the test settled on the owner map alone, which could look split while one process had yet to read the last member and would give units up on its next tick, so the "nothing moves once settled" check saw that handover (7 of 400 runs locally). Settled now also means every process has read every member and three ticks pass with no claim event (0 of 400). +- **The agent gate checks the alias and API forms of the PR actions it guards, and no longer blocks a command that only mentions one** (`.claude/hooks/agent-bash-gate.sh`, `.claude/hooks/{review-gate,hook-commands}.test.sh`, `scripts/githooks.test.sh`, `AGENTS.md`, `docs/src/content/docs/claude-code.md`), closing [#790](https://github.com/Wave-RF/WaveHouse/issues/790). `agent-bash-gate.sh` matched only the `gh pr` spellings, so each guarded action went through in another form. `gh pr new`, an alias of `gh pr create`, skipped the `--draft` check and the title lint. Through the REST API, `gh api …/pulls/<N>/reviews -f event=APPROVE` approved a PR, `gh api …/pulls -f head=… -f title=…` opened a non-draft one, `gh api …/requested_reviewers -f 'reviewers[]=…'` requested a reviewer (the check wanted `-X POST` or a literal `reviewers=`, though a field already makes `gh api` POST), and `gh api -X PUT …/pulls/<N>/merge` merged. Through GraphQL, `markPullRequestReadyForReview` marked a draft ready. The gate now gives each of these the same answer as its `gh pr` form. A `gh api` call counts as a write when it names a method other than GET or, naming none, sends fields. Opening a PR (`…/pulls`) needs `draft=true` and a `title=` that passes the PR-title lint, as does a retitle (`PATCH …/pulls/<N>`). A review with `event=APPROVE` or `event=REQUEST_CHANGES` is blocked. A write to `requested_reviewers`, `DELETE` included, is blocked. A merge (`PUT …/pulls/<N>/merge`, or GraphQL `mergePullRequest` or `enablePullRequestAutoMerge`) is blocked. In GraphQL, so are `markPullRequestReadyForReview`, `requestReviews`, an approving or changes-requesting `addPullRequestReview` or `submitPullRequestReview`, and a `createPullRequest` without `draft: true`. `gh pr merge` itself stays with the `deny` rule in `.claude/settings.json` and out of the gate, because a person merges with it from shell mode (`!`), and the Claude Code docs don't say whether a PreToolUse hook sees those commands. The `gh` checks used to read the line with its quoted strings removed and everything else kept, so they blocked a command that only mentioned a guarded one in a heredoc body or a comment (a `python3 - <<'EOF'` script with `# gh pr ready` in it), and missed one in a `$(…)` or handed to `bash -c`. They now read the commands the line runs through the gate's tokenizer, the view the push check already uses: a mention in a quoted string, heredoc body or comment doesn't count, while the code a shell runs, a `$(…)`, `bash -c '…'` (or `-lc` and the like), `eval`, or a heredoc a shell reads directly or through a pipe, is tokenized and judged the same way, read as written, quotes and heredoc bodies included, so a heredoc a shell reads inside a `$(…)` counts too. Each `gh` command is judged on its own words. A `gh api` call is judged by its endpoint, its first argument that isn't a flag, and by its fields, so a comment or review-thread reply whose body names an endpoint, a field or a mutation, or another call on the same line, doesn't count: `draft=`, `title=` and `event=` are read only from `-f`, `-F`, `--field` or `--raw-field` values, and GraphQL mutation names only from the `query=` field. A boolean flag set with `=` is read as gh reads it, so `--draft=true` counts as `--draft` and `--approve=false` doesn't approve. A line the tokenizer can't parse is refused when it mentions a `gh pr` or `gh api` command, since what it runs can't be told, and so is such a command left beyond the 64 pieces of nested code the gate follows. `gh -R <repo> pr …` (or `--repo`, before the command) counts as `gh pr …`; a shell given options before `-c` (`bash -euo pipefail -c '…'`, `bash --noprofile --norc -c '…'`) still hands its code over; and a GraphQL `query=` computed by a heredoc inside `$(…)` is matched against that heredoc. `gh pr create` and `gh pr new` are blocked when they name `--reviewer` or `--assignee` (or `-r`, `-a`), as `gh pr edit --add-reviewer` is. The `--title` of `gh pr create` and `gh pr edit` is now read from the tokens, so an unquoted title is linted too. The tokenizer now steps over plain text in a double-quoted string in one move rather than a character at a time: measured on an earlier revision of this change (a 100 KB inline `--body "…"`: 11 s with the old character loop, under 1 s after); not re-measured after the per-call rewrite. The gate still reads only the command line: fields sent through `--input` or `-F field=@file` go unseen. `test-review-gate`, `test-hook-commands` and `test-githooks` run the hooks a second time with `/bin/bash` first on `PATH` when that is a different bash, as macOS's bash 3.2 is, so a construct bash 3.2 rejects fails a test rather than leaving the gate to exit 1, which Claude Code doesn't treat as a block. +- **The Claude Code hooks no longer fail open when their scripts are missing** (`.claude/settings.json`, `.claude/hooks/hook-commands.test.sh` (new), `Makefile`, `AGENTS.md`, `docs/src/content/docs/{claude-code,development}.md`), closing [#787](https://github.com/Wave-RF/WaveHouse/issues/787): each hook ran `$CLAUDE_PROJECT_DIR/.claude/hooks/<script>.sh`, and `CLAUDE_PROJECT_DIR` is, in the Claude Code docs' words, ["the project root where the session started"](https://code.claude.com/docs/en/hooks#reference-scripts-by-path). Once that checkout was removed, or a script went missing or lost its executable bit, the shell exited 127 or 126, which Claude Code treats as a non-blocking error, so `agent-bash-gate.sh` stopped gating pushes and PR changes and `review-marker.sh` stopped writing markers, and nothing said so. Each command is now a short guard that `exec`s its script, unchanged, when the script is there and executable, and otherwise acts by event. The PreToolUse guard blocks (exit 2) any command that contains the words `git` and `push`; `gh` and `pr` with one of `create`, `new`, `edit`, `ready`, `review` or `merge`; or `gh` and `api` with `pulls` or `graphql`, which covers every call to the pull-request or GraphQL API, reads included. It also blocks every command when there is no `jq` to read it with. Other commands go through, so the session can still work. The SubagentStop guard reports the missing script with exit 1, since exit 2 there keeps the subagent running, and the push gate then finds no marker. The PostToolUse guards report it with exit 2, which shows Claude the message after an edit that has already happened. Each message says to restart Claude Code from a live checkout. The match is crude on purpose, a guard against accidents like the gate it stands in for. A project directory whose path has a space in it now works too: the bare path was split. `make verify` gains `test-hook-commands`, which reads every command from the real `settings.json` and runs it under `sh -c` (and `dash` and `bash` when present) with its script present, missing and not executable. +- **A worktree whose branch has no Makefile can commit, and the git hooks say when `scripts/ci-marker.sh` is missing** (`.githooks/{pre-commit,pre-push}`, `scripts/githooks.test.sh` (new), `Makefile`, `AGENTS.md`, `docs/src/content/docs/{claude-code,development}.md`), closing [#678](https://github.com/Wave-RF/WaveHouse/issues/678): with an absolute `core.hooksPath`, git runs one checkout's `.githooks/` in every worktree of the repository, including one on an orphan branch that keeps a tool's state and no code. There `pre-commit` couldn't find `scripts/ci-marker.sh`, hid the error behind `2>/dev/null`, and ran `make verify` with no Makefile, which failed with `No rule to make target 'verify'`, so nothing could be committed. A worktree with no Makefile now has nothing to verify and commits; a missing or failing `scripts/ci-marker.sh` is reported instead of hidden, and `make verify` runs as before. `pre-push` blocked the same branches with a misleading "can't resolve tree"; it now lets through a pushed commit that has no Makefile, which holds no code to validate, and blocks one that has a Makefile but is pushed from a worktree without `scripts/ci-marker.sh`, saying so. The relative `.githooks` that `make tools` sets is resolved in each worktree, so there a worktree whose branch has no `.githooks/` runs no hooks at all; the hooks now behave correctly under either setting. `make verify` gains `test-githooks`, which commits and pushes through the real hooks from scratch worktrees with and without code, and `make lint-sh` now shellchecks `.githooks/`. - **`scripts/skip-pre-push-review.sh` no longer refuses, now and then, to skip the first reviewer the manifest lists** (`scripts/skip-pre-push-review.sh`, `.claude/hooks/review-gate.test.sh`), closing [#767](https://github.com/Wave-RF/WaveHouse/issues/767): it checked the name with `bash scripts/pre-push-reviewers.sh | grep -Fxq` under `pipefail`. Bash writes each name in its own `write()`, so `grep -q` could match `pre-push-reviewer` on the first line and exit before the second write, which then died of `SIGPIPE`; `pipefail` reported that as no match, and the script refused with "'pre-push-reviewer' is not a reviewer". Whether it happened was up to the scheduler, so `make test-review-gate` failed intermittently in CI, more often on a busy runner, and its log showed only that no marker was written, because the test discarded the script's stderr. The script now reads the whole list before matching, and does the same for its advisory `git diff --name-only | grep -qv` check; that check also resolves its base the way the push gate does (`origin/main`, else `main`), which its comment already claimed. The test now prints the script's stderr when a skip fails, and adds a case whose manifest pauses after each name, which the old script failed every time. - **The multi-tenant docs say how a tenant folder is added by hand, where `X-Tenant-ID` is resolved, and when a stream can see the tenant `404`** (`docs/src/content/docs/{deployment,api}.md`, `docs/src/content/docs/sdk/{reference,streaming,admin}.md`, `docs/src/content/docs/settings-directory.mdx`): story 11 of [#583](https://github.com/Wave-RF/WaveHouse/issues/583). `wavehouse bootstrap` seeds one folder of a nested directory when given the folder's path, and the new folder is adopted by a whole-directory reload, not a reload by name; the API page's Authentication section now says the tenant is resolved from the header before the token is read. The SDK pages said a stream sees the `400`/`404` of tenant resolution only "when you send `X-Tenant-ID`"; a stream that sends none is tenant `0`'s, which over a nested directory with no `0` folder is the `404` ([#641](https://github.com/Wave-RF/WaveHouse/issues/641)). The DLQ example on the SDK admin page declared `data` twice in one block and showed a `0` count the server never returns, the `?table=` row said the filter narrows `total`, and the dead-letter section called the unreadable-envelope exception "new in this release" ([#676](https://github.com/Wave-RF/WaveHouse/issues/676)). - **`make ci` no longer kills healthy test runs at the `go test` time limits** (`Makefile`, `docs/src/content/docs/development.md`): its parallel phase runs the unit suite beside the lint and build jobs on every core, where `internal/app` and `internal/mq`, about 10s each alone, run past the suite's 15s per-package budget, so that phase now gives the suite 60s (`CI_UNIT_TIMEOUT`). `make test-unit` and the unit job in CI, which run the suite on its own, keep the 15s (`UNIT_TIMEOUT`). The integration suite's limit goes from 480s to 900s (`INTEGRATION_TIMEOUT`): `tests/integration` takes about 6m on a CI runner and up to 8m under Docker Desktop, which 480s left half a minute. Each limit is a Makefile variable, so one can be raised for a run (`make ci CI_UNIT_TIMEOUT=90s`). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b473f715..5c5e2af9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ Open a [feature request issue](https://github.com/Wave-RF/WaveHouse/issues/new?t make ci # full local pipeline: verify + builds + all test suites (Docker required) ``` - The pre-push hook (installed by `make tools`) blocks a push until the tree has been validated locally: a code change needs `make ci`, a docs/prose-only change needs only `make verify` (the same split CI makes). `make lint` / `make test` / `make build` are fast inner-loop subsets. + The pre-push hook (installed by `make tools`) blocks a push until the tree has been validated: a code change needs `make ci`, run locally or on another machine with `make ci-remote`, and a docs/prose-only change needs only `make verify` (the same split CI makes). `make lint` / `make test` / `make build` are fast inner-loop subsets. 2. Write tests for new functionality. Unit tests go alongside the code in `internal/`. Integration tests carry the `//go:build integration` tag and go in `tests/integration/`, or beside the package when they test one package against its own external server (e.g. `internal/cache/redis_integration_test.go`), with the package added to a part of the integration suite in the `Makefile` (`make test-integration` fails while it is in none). A test that must import NATS, which only `internal/mq` may do, goes in `internal/mq/natsspike`, or in `internal/mq` itself with the `integration` tag when it needs the package's internals (the external NATS broker's tests and the NATS KV lease tests, which `make test-integration` selects by that tag). diff --git a/Makefile b/Makefile index 2367d29f..9d6184ab 100644 --- a/Makefile +++ b/Makefile @@ -96,6 +96,7 @@ ARCH := $(shell uname -m | sed -e 's/x86_64/amd64/' -e 's/aarch64/arm64/') # LDFLAGS="-s -w" extra ldflags (e.g. force-strip a local build) # LIMIT=50 top-N for `make dep-cut` # JOBS=4 parallel slots for the -j fan-outs (default: CPU count) +# HOST=box KEEP=1 REMOTE_DIR=d where `make ci-remote` runs (see that target) # # Add binaries here as the project grows (e.g., wavehouse-api, wavehouse-worker). BINARIES := wavehouse @@ -428,10 +429,11 @@ lint-prose: $(MISSPELL) $(call run,misspell (US spelling),$(MISSPELL) -locale US -source text -error $(DOCS_PROSE),run make fix to auto-correct) # lint-sh: shellcheck over every tracked shell script (scripts/, hooks, -# docs tooling). -x follows `source`d files; -P SCRIPTDIR resolves -# `# shellcheck source=` directives relative to the sourcing script, not -# the cwd. Lazily expanded so the ls-files only runs when the target does. -SHELL_SOURCES = $(shell git ls-files '*.sh') +# docs tooling), the extensionless git hooks in .githooks/ included. -x follows +# `source`d files; -P SCRIPTDIR resolves `# shellcheck source=` directives +# relative to the sourcing script, not the cwd. Lazily expanded so the +# ls-files only runs when the target does. +SHELL_SOURCES = $(shell git ls-files '*.sh' '.githooks/*') .PHONY: lint-sh lint-sh: $(SHELLCHECK) $(call run,shellcheck,$(SHELLCHECK) -x -P SCRIPTDIR $(SHELL_SOURCES),) @@ -511,6 +513,31 @@ test-prune-pr-build-cache: test-review-gate: $(call run,review-gate test,.claude/hooks/review-gate.test.sh,) +# test-remote-ci: drive scripts/ci/remote-ci.sh through stand-in ssh and docker +# commands in a scratch repository: only a passing run of exactly HEAD's tree, +# on a worktree still clean afterwards, may write the make-ci marker. Also +# checks the pre-push hook's line for that marker. A verify leaf, same as +# test-classify-paths. +.PHONY: test-remote-ci +test-remote-ci: + $(call run,remote-ci test,scripts/ci/remote-ci.test.sh,) + +# test-githooks: commit and push through the .githooks/ hooks from scratch +# worktrees with and without code: a worktree whose branch has no Makefile must +# commit, and code must still need its markers. A verify leaf, same as +# test-review-gate. +.PHONY: test-githooks +test-githooks: + $(call run,githooks test,scripts/githooks.test.sh,) + +# test-hook-commands: run each hook command in .claude/settings.json the way +# Claude Code does, with its script present and with it missing: a missing +# push gate must block a push, not exit 127 (which Claude Code lets through). +# A verify leaf, same as test-review-gate. +.PHONY: test-hook-commands +test-hook-commands: + $(call run,hook-commands test,.claude/hooks/hook-commands.test.sh,) + .PHONY: vulncheck vulncheck: go-mod-download ## Run govulncheck -scan package (V=1: symbol-level scan with example traces) ifdef V @@ -601,13 +628,13 @@ fix-prose: $(MISSPELL) # slowest tool, not the slowest *group* (e.g. golangci no longer drags Biome + # markdownlint along behind it). # -# Leaves (20): tidy, fmt-go (gofumpt), lint-go (golangci), vulncheck on the Go +# Leaves (23): tidy, fmt-go (gofumpt), lint-go (golangci), vulncheck on the Go # side; lint-ts (biome check) + lint-md (markdownlint) + lint-prose (misspell, # docs spelling) + test-md-rules (node --test over the WH001/WH002 fixtures) # for JS/TS + Markdown + prose; lint-sh (shellcheck), lint-gha (actionlint), # test-classify-paths, test-release-channel, test-go-toolchain, check-dockerfile-go, test-tagged-tests, -# test-integration-parts, test-prune-pr-build-cache and test-review-gate for the -# tooling; +# test-integration-parts, test-prune-pr-build-cache, test-review-gate, +# test-remote-ci, test-githooks and test-hook-commands for the tooling; # check-docs (astro check — the only leaf that writes, to docs/.astro/, and # nothing else touches it) and typecheck-ts (tsc --noEmit). It runs lint-ts # (`biome check`) but NOT fmt-ts (`biome format`) — check already covers @@ -621,7 +648,7 @@ verify: ## Run all static checks across the repo (Go + TS + docs, parallelized) @printf "$(GREEN)$(BOLD)✔ All static checks passed$(RESET)\n" .PHONY: verify-parallel -verify-parallel: tidy fmt-go lint-go lint-ts lint-md lint-prose lint-sh lint-gha test-classify-paths test-md-rules test-release-channel test-go-toolchain check-dockerfile-go test-tagged-tests test-integration-parts test-prune-pr-build-cache test-review-gate vulncheck check-docs typecheck-ts +verify-parallel: tidy fmt-go lint-go lint-ts lint-md lint-prose lint-sh lint-gha test-classify-paths test-md-rules test-release-channel test-go-toolchain check-dockerfile-go test-tagged-tests test-integration-parts test-prune-pr-build-cache test-review-gate test-remote-ci test-githooks test-hook-commands vulncheck check-docs typecheck-ts # typecheck-ts: tsc --noEmit on the SDK. Its own target (was inline in verify's # recipe) so it can run as a parallel leaf of verify-parallel. @@ -1018,6 +1045,16 @@ ci: ## Full pipeline — parallel checks, then sequential heavy suites + coverag @scripts/ci-marker.sh write @echo "$(GREEN)$(BOLD)✔ All CI checks passed$(RESET)" +# ci-remote: run `make ci` for HEAD on another machine, an ssh destination or +# docker:<container>, and write the same markers here if it passed there for +# exactly this tree. Host requirements and the checks: scripts/ci/remote-ci.sh. +HOST ?= +KEEP ?= +REMOTE_DIR ?= +.PHONY: ci-remote +ci-remote: ## Run `make ci` for HEAD on HOST=<ssh destination> or HOST=docker:<container> (KEEP=1, REMOTE_DIR=…) + @KEEP='$(KEEP)' REMOTE_DIR='$(REMOTE_DIR)' scripts/ci/remote-ci.sh '$(HOST)' + ##@ Release # Releases are cut by pushing ONE tag — no version bump, no commit, no release diff --git a/docs/src/content/docs/claude-code.md b/docs/src/content/docs/claude-code.md index 75d927e0..39f28eb9 100644 --- a/docs/src/content/docs/claude-code.md +++ b/docs/src/content/docs/claude-code.md @@ -24,7 +24,7 @@ If you're new to Claude Code itself, the [official docs](https://code.claude.com | Layer | Lives in | Applies to | Purpose | | ----- | -------- | ---------- | ------- | | **Git hooks** | `.githooks/` (installed by `make tools`) | Humans + Claude uniformly | Hard enforcement: `make verify` on commit; before push, `make ci` for a code change or `make verify` for a docs-only one (classifier-gated) | -| **Claude Code agent gate** | `.claude/hooks/agent-bash-gate.sh` (PreToolUse Bash) + `.claude/settings.json` deny rules | Agents only | Catches accidental violations of [Agent PR Discipline](#agent-pr-discipline): drafts only, no human reviewer adds, a marker (from a review or a logged skip) from every reviewer in `scripts/pre-push-reviewers.sh` required for the commit each refspec publishes (its tip, or HEAD with no refspec) when `main` doesn't have it yet, judged in the worktree the push runs in; PR title linted via `scripts/lint-pr-title.sh` on `gh pr create` / `gh pr edit --title` | +| **Claude Code agent gate** | `.claude/hooks/agent-bash-gate.sh` (PreToolUse Bash) + `.claude/settings.json` deny rules | Agents only | Catches accidental violations of [Agent PR Discipline](#agent-pr-discipline): drafts only, no human reviewer adds, a marker (from a review or a logged skip) from every reviewer in `scripts/pre-push-reviewers.sh` required for the commit each refspec publishes (its tip, or HEAD with no refspec) when `main` doesn't have it yet, judged in the worktree the push runs in; PR title linted via `scripts/lint-pr-title.sh` on `gh pr create` / `gh pr new` / `gh pr edit --title`; the `gh api` REST and GraphQL forms of these, and of a merge, checked too | | **Claude Code ergonomic hooks** | `.claude/hooks/gofumpt-on-save.sh` + `.claude/hooks/markdown-on-save.sh` (PostToolUse Edit/Write/MultiEdit), `.claude/hooks/review-marker.sh` (SubagentStop) | Claude only | gofumpt / markdown-on-save: auto-format on file edits (humans get this from their IDE). review-marker: on a reviewer's `VERDICT: ship_it`, writes that reviewer's `tmp/<name>-passed-<sha>` marker for the commit its `REVIEWED:` line names | | **Claude Code skills / agents / commands** | `.claude/skills/`, `.claude/agents/`, `.claude/commands/` | Claude only (when relevant) | Workflow guidance and on-demand helpers — not gates | @@ -36,23 +36,26 @@ Two scripts, both committed to the repo: | Hook | Behavior | | ---- | -------- | -| `pre-commit` | Runs `make verify` (the static checks listed in [Development → Makefile Targets](/development#makefile-targets), ~30s) — **blocks on failure**. Skipped if `make ci` or `make verify` already ran for the current tree state (cached via `scripts/ci-marker.sh`). | -| `pre-push` | Scales the bar to the change set (same classifier CI uses, `scripts/classify-paths.sh`): a **code** change requires the `make ci` marker (`tmp/ci-passed-tree-<TREE-sha>`); a **docs/prose-only** push requires only the `make verify` marker (`tmp/verify-passed-tree-<TREE-sha>`) — CI skips the Go/SDK suites for those too. **Blocks** if the required marker is absent. Fail-closed: an unclassifiable push falls back to requiring `make ci`. | +| `pre-commit` | Runs `make verify` (the static checks listed in [Development → Makefile Targets](/development#makefile-targets), ~30s) — **blocks on failure**. Skipped if `make ci` or `make verify` already ran for the current tree state (cached via `scripts/ci-marker.sh`). A worktree with no `Makefile` has nothing to verify and commits; a missing or failing `scripts/ci-marker.sh` is reported and `make verify` runs. | +| `pre-push` | Scales the bar to the change set (same classifier CI uses, `scripts/classify-paths.sh`): a **code** change requires the `make ci` marker (`tmp/ci-passed-tree-<TREE-sha>`, from `make ci` or `make ci-remote`); a **docs/prose-only** push requires only the `make verify` marker (`tmp/verify-passed-tree-<TREE-sha>`) — CI skips the Go/SDK suites for those too. **Blocks** if the required marker is absent. Fail-closed: an unclassifiable push falls back to requiring `make ci`. A pushed commit with no `Makefile` goes through, having no code to validate; one with a `Makefile`, pushed from a worktree without `scripts/ci-marker.sh`, is blocked. | `--no-verify` is for intentional WIP / draft pushes. Agents should not use it — policy in AGENTS.md §"Agent PR Discipline", not regex-enforced. Both markers are tree-keyed so commit-then-push works without a re-run when the tree is unchanged. `make ci` / `make verify` skip the marker write when `$CI` is set (CI runners don't push). Shared logic lives in `scripts/ci-marker.sh`. +`make tools` sets `core.hooksPath` to the relative `.githooks`, which git resolves in each worktree: each worktree runs its own branch's hooks, and one whose branch has no `.githooks/` runs none. An absolute `core.hooksPath` runs one checkout's hooks in every worktree, which is why the hooks handle a worktree with no code. `make test-githooks` (part of `make verify`) commits and pushes through the real hooks from scratch worktrees with and without code. + ## What's in `.claude/` and `.config/` | Path | Purpose | | ---- | ------- | -| `.claude/settings.json` | Team-wide: `deny` permissions (force-push / git reset --hard / filter-branch / update-ref -d, gh pr merge / ready / approve / request-changes, gh repo/release delete, gh secret delete, gh workflow disable, rm -rf / sudo rm), all four hooks wired | +| `.claude/settings.json` | Team-wide: `deny` permissions (force-push / git reset --hard / filter-branch / update-ref -d, gh pr merge / ready / approve / request-changes, gh repo/release delete, gh secret delete, gh workflow disable, rm -rf / sudo rm), all four hooks wired, each through a guard that runs its script when it's there and fails closed when it isn't (see [When a hook script is missing](#when-a-hook-script-is-missing)) | | `.claude/hooks/gofumpt-on-save.sh` | PostToolUse Edit/Write/MultiEdit: auto-formats `.go` files | | `.claude/hooks/markdown-on-save.sh` | PostToolUse Edit/Write/MultiEdit: repairs MDX fence structure in `.mdx`, applies markdownlint `--fix` (WH001 unwrapping) in `.md` only, and runs misspell over docs prose in both | -| `.claude/hooks/agent-bash-gate.sh` | PreToolUse Bash: catches accidental Agent PR Discipline violations (drafts only, no human reviewer adds, a marker (from a review or a logged skip) from every reviewer in `scripts/pre-push-reviewers.sh` required for the commit each refspec publishes (its tip, or HEAD with no refspec) when `origin/main` (local `main` if there's no `origin/main`) doesn't have it yet; PR title linted via `scripts/lint-pr-title.sh` on `gh pr create` / `gh pr edit --title`). A push is judged where it runs (the tool call's `cwd`, a `cd` or `pushd` before it, `git -C`), and a marker counts from any worktree of the repository. It blocks a push it can't follow (a computed path or refspec, `GIT_DIR` or `GIT_WORK_TREE` set earlier on the line, `--all` or a glob refspec, a push behind a wrapper or in a command substitution, code handed to `bash -c`, `eval` or a heredoc or here-string read by a shell) and one that follows a HEAD-moving git command on the same line, run directly or not. It reads only the command line, so it catches the common accidental forms, not deliberate evasion; it doesn't follow a script that pushes, a `$(…)` inside an unquoted heredoc body, a heredoc read by a shell inside `$(…)`, a shell behind a wrapper or one it doesn't know (`fish -c`), code piped into a shell, `git push --tags`, or git run by path or through an alias. A line it can't parse, such as one with an apostrophe in a comment inside `$(…)`, is refused | +| `.claude/hooks/agent-bash-gate.sh` | PreToolUse Bash: catches accidental Agent PR Discipline violations (drafts only, no human reviewer adds, a marker (from a review or a logged skip) from every reviewer in `scripts/pre-push-reviewers.sh` required for the commit each refspec publishes (its tip, or HEAD with no refspec) when `origin/main` (local `main` if there's no `origin/main`) doesn't have it yet; PR title linted via `scripts/lint-pr-title.sh` on `gh pr create` / `gh pr new` / `gh pr edit --title`; the `gh api` REST and GraphQL forms of these, and of a merge, checked too, reading the command line only, so not fields sent through `--input` or `-F field=@file`). A gh command only mentioned in a quoted string, a heredoc body or a comment doesn't count, unless a shell runs it. A push is judged where it runs (the tool call's `cwd`, a `cd` or `pushd` before it, `git -C`), and a marker counts from any worktree of the repository. It blocks a push it can't follow (a computed path or refspec, `GIT_DIR` or `GIT_WORK_TREE` set earlier on the line, `--all` or a glob refspec, a push behind a wrapper or in a command substitution, code handed to `bash -c`, `eval` or a heredoc or here-string read by a shell) and one that follows a HEAD-moving git command on the same line, run directly or not. It reads only the command line, so it catches the common accidental forms, not deliberate evasion; it doesn't follow a script that pushes, a `$(…)` inside an unquoted heredoc body, a heredoc read by a shell inside `$(…)`, a shell behind a wrapper or one it doesn't know (`fish -c`), code piped into a shell, `git push --tags`, or git run by path or through an alias. A line it can't parse, such as one with an apostrophe in a comment inside `$(…)`, is refused when it mentions a push or a `gh pr` / `gh api` command. It follows 64 pieces of code a shell runs; a `gh pr` / `gh api` command left beyond them is refused unjudged | | `.claude/hooks/review-marker.sh` | SubagentStop: on a reviewer's `VERDICT: ship_it`, writes its `tmp/<name>-passed-<sha>` marker for the commit named on the report's `REVIEWED:` line, into the worktree on that commit, and only if that worktree's HEAD stayed on it for the whole review (its reflog against the subagent transcript's first timestamp). The reviewer set comes from `scripts/pre-push-reviewers.sh`; filters by `agent_type` in-script. Reads the report from `.last_assistant_message`, or, when that has no `VERDICT:` line or says `ship_it` without `REVIEWED:`, from the hand-back message in the subagent's transcript; a final `iterate` or `block` stands. An abbreviated sha counts if it names exactly one commit. Logs every decision to `tmp/review-marker.log` | | `.claude/hooks/review-gate.test.sh` | Test for the two review hooks: feeds them synthetic events in a scratch repository with sibling worktrees (`make test-review-gate`, part of `make verify`) | +| `.claude/hooks/hook-commands.test.sh` | Test for the hook commands in `.claude/settings.json`: runs each one under `sh -c` as Claude Code does, with its script present, missing and not executable (`make test-hook-commands`, part of `make verify`) | | `.claude/commands/cover.md` | `/cover [suite]` — suite dispatch + coverage threshold analysis | | `.claude/commands/docs-review.md` | `/docs-review [path\|all]` — launches the `docs-reviewer` subagent. No-arg = the gating pre-push docs review; a path/`all` = advisory | | `.claude/commands/prepush.md` | `/prepush [all]` — the pre-push gate: reads `scripts/pre-push-reviewers.sh`, runs the reviewers the change needs in parallel (fresh context), skips the rest on the record (`scripts/skip-pre-push-review.sh`), loops to `ship_it`. `all` forces the full set | @@ -103,8 +106,9 @@ To add a skill: create `.claude/skills/<name>/SKILL.md` with frontmatter `name` Agents (Claude Code etc.) have additional gating beyond what humans face — enforced by `.claude/hooks/agent-bash-gate.sh` (PreToolUse Bash) + deny rules in `.claude/settings.json`. Humans keep full git/gh affordances; agents have these extra constraints: -- **Drafts only.** `gh pr create` must include `--draft`. Only humans transition draft → ready (`gh pr ready` is blocked), approve (`gh pr review --approve` is blocked), or request changes (`gh pr review --request-changes` is blocked). The gate also lints the PR title on `gh pr create` / `gh pr edit --title` via `scripts/lint-pr-title.sh` — the same rule as the required `CI` check's `PR title` job, so a malformed title is caught before the PR exists (fail-open when no quoted title is parseable). -- **No human reviewer assignment.** `gh pr edit --add-reviewer / --add-assignee` and `POST /requested_reviewers` are blocked. GitHub assigns reviewers natively — the `required_reviewers` ruleset rule requests the `@Wave-RF/wavehouse-admins` team and the team's code-review assignment picks the member; humans handle the rest. +- **Drafts only.** `gh pr create` (or its alias `gh pr new`) must include `--draft`. Only humans transition draft → ready (`gh pr ready` is blocked), approve (`gh pr review --approve` is blocked), or request changes (`gh pr review --request-changes` is blocked). The gate also lints the PR title on `gh pr create` / `gh pr new` / `gh pr edit --title` via `scripts/lint-pr-title.sh` — the same rule as the required `CI` check's `PR title` job, so a malformed title is caught before the PR exists (fail-open when the title is missing or computed). +- **The API forms count too.** Through `gh api`, opening a PR (`…/pulls`) needs `-F draft=true` and a title that passes the lint, and a review with `event=APPROVE` or `event=REQUEST_CHANGES` is blocked. Through GraphQL, `createPullRequest` needs `draft: true`, and `markPullRequestReadyForReview`, an approving or changes-requesting `addPullRequestReview` / `submitPullRequestReview`, and `requestReviews` are blocked. A merge through the API (`PUT …/pulls/<N>/merge`, `mergePullRequest`, `enablePullRequestAutoMerge`) is blocked. `gh pr merge` itself is left to the `deny` rule in `.claude/settings.json`, because a person merges with it from shell mode (`!`). Each `gh api` call is judged by its own endpoint (its first argument that isn't a flag) and its own fields, so a comment or review-thread reply whose body names an endpoint doesn't count, and neither does another call on the same line. Code a shell runs (`$(…)`, `bash -c '…'` with or without options before `-c`, `eval`, a heredoc a shell reads) is judged the same way, and so is a GraphQL query computed by a heredoc inside `$(…)`. `gh -R <repo> pr …` (or `--repo`) counts as `gh pr …`. The gate reads the command line only, so fields sent through `--input` or `-F field=@file` go unseen. +- **No human reviewer assignment.** `gh pr edit --add-reviewer / --add-assignee`, `--reviewer` / `--assignee` (or `-r` / `-a`) on `gh pr create` / `gh pr new`, and any `gh api` write to `…/requested_reviewers` are blocked. GitHub assigns reviewers natively — the `required_reviewers` ruleset rule requests the `@Wave-RF/wavehouse-admins` team and the team's code-review assignment picks the member; humans handle the rest. - **Bot re-triggers via comments.** Agents CAN mention bots in PR comments to re-trigger reviews — `@coderabbitai review`, etc. This goes through `gh pr comment` (allowed), not the reviewer API. - **Pre-push review required on PR branches.** Before `git push` to a branch with commits ahead of `main`, the agent runs the reviewers in `scripts/pre-push-reviewers.sh` (today: `pre-push-reviewer` for code, `docs-reviewer` for docs prose + code↔docs sync) that the change needs — in fresh context, in parallel — and **skips** any with nothing to do via `scripts/skip-pre-push-review.sh <name> "<reason>"` (a logged skip that satisfies the marker, so a docs typo doesn't pay for a full code review). `/prepush` does all of this. `ship_it` requires zero findings at any severity — any `[MUST]` / `[SHOULD]` / `[MAY]` forces iterate; fix and re-invoke (always fresh context) until clean. The push gate requires a marker — from a `ship_it` or a logged skip — from every listed reviewer for the commit each refspec publishes (its tip, or HEAD with no refspec), judged in the worktree the push runs in, and echoes the skips at push time. A `ship_it` marks only the commit the reviewer read, so don't commit in a worktree while its reviewers run. - **Don't bypass.** `--no-verify` and hand-writing markers are policy violations, not regex-blocked. An agent that wants to bypass can edit the gate itself — trust the policy in AGENTS.md §"Agent PR Discipline". Markers come from `make ci`, the `review-marker.sh` hook, and `scripts/skip-pre-push-review.sh` (logged skips); nothing else. @@ -112,6 +116,18 @@ Agents (Claude Code etc.) have additional gating beyond what humans face — enf Full ruleset and rationale: AGENTS.md §"Agent PR Discipline". +### When a hook script is missing + +Each hook in `.claude/settings.json` names its script under `$CLAUDE_PROJECT_DIR`, which the Claude Code docs define as ["the project root where the session started"](https://code.claude.com/docs/en/hooks#reference-scripts-by-path), and which stays put when the session enters a worktree. If that checkout is removed while the session runs, or a script goes missing or loses its executable bit, a bare path would make the shell exit 127, and Claude Code treats any exit code but 2 as a [non-blocking error](https://code.claude.com/docs/en/hooks#other-exit-codes): the push gate would stop gating without a word. So each command is a short guard that `exec`s the script when it's there and executable, unchanged, and otherwise acts by event: + +| Event | Script | With the script missing | +| ----- | ------ | ----------------------- | +| PreToolUse (Bash) | `agent-bash-gate.sh` | Blocks (exit 2) a command that contains the words `git` and `push`; `gh` and `pr` with one of `create`, `new`, `edit`, `ready`, `review` or `merge`; or `gh` and `api` with `pulls` or `graphql`, which covers every call to the pull-request or GraphQL API, reads included. It also blocks every command when there is no `jq` to read it with. Other commands go through, so the session can still work | +| SubagentStop | `review-marker.sh` | Reports it on stderr with exit 1. Exit 2 [would keep the subagent running](https://code.claude.com/docs/en/hooks#exit-code-2-behavior-per-event). A reviewer's `ship_it` writes no marker, so the push gate blocks for lack of one | +| PostToolUse | `gofumpt-on-save.sh`, `markdown-on-save.sh` | Reports it with exit 2, which shows Claude the message; the edit has already happened | + +Every message names the missing script and says to restart Claude Code from a live checkout. The PreToolUse match is crude on purpose: like the gate, it guards against accidents, not deliberate workarounds, and it errs toward blocking. + ## Worktrunk integration We use [worktrunk](https://worktrunk.dev) as the recommended worktree manager. It creates real `git worktree add` directories, so the committed `.claude/` config Just Works in every worktree. diff --git a/docs/src/content/docs/development.md b/docs/src/content/docs/development.md index 8a8069df..d9eaa3b2 100644 --- a/docs/src/content/docs/development.md +++ b/docs/src/content/docs/development.md @@ -19,7 +19,7 @@ You need these on your `PATH` before any `make` recipe will work end-to-end: | **Docker** *(or Podman)* | Engine 20.10+ with the Compose **v2** plugin (`docker compose`, no hyphen) | Compose stacks under `deployments/compose/`; the E2E and integration suites boot ClickHouse and a Redis via testcontainers (no compose file), the integration suite also dynamodb-local, and the integration suite also runs the shared cache backend against Redis, Valkey, Dragonfly (pulled from `docker.dragonflydb.io`) and a one-node Redis Cluster | [Docker Desktop](https://docs.docker.com/get-docker/), [colima](https://github.com/abiosoft/colima), or [Podman](https://podman.io) with `podman-compose` / the `podman compose` plugin. The testcontainers Go library also honors `DOCKER_HOST` for rootless Podman setups | | **Node.js** | 22 LTS — pinned via `.nvmrc` at the repo root | Runtime for pnpm and the Vitest suites. Pinned to match CI (`setup-node` uses 22) and to avoid Node-major surprises; older Vitest versions in this repo were known to crash on Node 26 with a V8 heap-allocation abort | [nodejs.org](https://nodejs.org/) or `nvm use` / `fnm use` / `volta` (all read `.nvmrc`) | | **pnpm** | 11.21+ (pinned via `packageManager` in the root `package.json`) | Package manager for the TypeScript SDK, E2E test harness, and docs site (managed as a single pnpm workspace from the repo root); `make tools`, `make verify`, `make lint`, `make fmt`, `make fix`, `make ci`, `make build-ts`, `make dev-ts`, `make test-ts`, `make test-e2e`, `make test-all`, `make check-docs`, `make build-docs`, `make build-all`, `make dev-docs`, `make preview-docs` all shell out to `pnpm` (`make tools` itself runs `pnpm install`, and `make verify` is what the pre-commit hook runs, so a Go-only contributor needs pnpm too) | `corepack enable && corepack prepare pnpm@11.21.0 --activate` (recommended), or `npm i -g pnpm` | -| **git** + **curl** + **jq** | any recent | `git` for source + version metadata in builds; `curl` is used by the Makefile to fetch the pinned `golangci-lint` and `shellcheck` binaries into `.bin/` and, for `make build-ts`, the oldest Node that `engines.node` admits; `jq` is needed by `make verify` (the review-gate hook tests) and by the Claude Code hooks | usually preinstalled (`jq` ships with macOS 15+); otherwise `apt install jq` / `brew install jq` | +| **git** + **curl** + **jq** | any recent | `git` for source + version metadata in builds; `curl` is used by the Makefile to fetch the pinned `golangci-lint` and `shellcheck` binaries into `.bin/` and, for `make build-ts`, the oldest Node that `engines.node` admits; `jq` is needed by `make verify` (the hook tests) and by the Claude Code hooks | usually preinstalled (`jq` ships with macOS 15+); otherwise `apt install jq` / `brew install jq` | ### Auto-installed by `make tools` @@ -340,6 +340,22 @@ Each test target writes `covdata` to `tmp/coverage/<suite>/data/`, renders a tex **Build cache across worktrees**: `make` exports `GOFLAGS=-trimpath` (your own `GOFLAGS` are kept), so compiled packages don't embed the checkout's path and every worktree of the same source shares one build cache entry per package and variant, instead of building its own. Opt out with `GOFLAGS=-trimpath=false make …`. `make build` is the exception: its debug binary keeps absolute source paths, so a debugger finds the files. Release and coverage binaries and test binaries built through `make` are trimmed; their frames read `github.com/Wave-RF/WaveHouse/internal/…` (standard-library frames are plain import paths such as `runtime/proc.go`), so map the module path to your checkout, for example, at the `(dlv)` prompt, `config substitute-path github.com/Wave-RF/WaveHouse /absolute/path/to/your/checkout` (or a `substitute-path` entry with `from` and `to` in `~/.config/dlv/config.yml`, or your editor's `substitutePath`). Code that needs a file from the repository finds it from the working directory, never from `runtime.Caller`, whose paths are module-relative under `-trimpath`. +### Running `make ci` on another machine + +`make ci-remote HOST=<host>` runs `make ci` for your `HEAD` commit on another machine and streams its output back, so the heavy suites can run off your laptop, or on another OS or architecture than yours (CI runs on Linux amd64). + +```bash +make ci-remote HOST=build-box # any ssh destination: user@host, a ~/.ssh/config alias +make ci-remote HOST=docker:wavehouse-dev # a running local container or devcontainer +make ci-remote HOST=build-box KEEP=1 REMOTE_DIR=/scratch/wavehouse-ci +``` + +Only the commit travels, so the worktree must be clean: `HEAD` is streamed as a `git bundle` over the ssh or `docker exec` connection, and nothing is pushed. The host checks it out in a fresh directory under `REMOTE_DIR` (default `.cache/wavehouse-ci`; a relative path is under the host's home), runs `make tools && make ci` there, and removes the directory afterwards unless `KEEP=1`. After the local `==> make ci on …` line, the first line the host prints is its `uname -sm`, and each run keeps a copy of its log in `tmp/ci-remote-<tree>-<run>.log`, so two runs of one tree, say on two architectures at once, don't overwrite each other. If `make ci-remote` is stopped, by Ctrl-C or a signal, the host stops its `make ci` within a second or two: the host's end of the connection stays open only while the local command runs, and the host stops the run when it closes. + +The host needs what a local `make ci` needs: bash, git, curl, jq, GNU Make 4+, Go, Node and pnpm from [Prerequisites](#prerequisites), network access for `make tools`, and a Docker daemon for the integration and e2e suites. Over ssh the commands run in a non-interactive login shell, so the tools must be on the `PATH` the host's profile sets; in a container they must be on the container's own `PATH`, and the container must reach a Docker daemon (a mounted socket or Docker-in-Docker). If `make ci` runs inside a container on the host's network, testcontainers may try to reach the suites' containers through the Docker bridge gateway and time out; setting `TESTCONTAINERS_HOST_OVERRIDE=localhost` in the host's environment points it at the published ports instead. A Linux host without Chromium's system libraries needs them installed once for the docs build, as Prerequisites describes (`pnpm --filter wavehouse-docs exec playwright install-deps chromium`). + +**How the pre-push gate treats it.** The gate's meaning doesn't change: a code push still needs `tmp/ci-passed-tree-<tree>`, which says `make ci` passed for exactly that tree. `make ci-remote` writes it (and the verify marker) only if the host exits 0, the tree the host reports having checked out equals your `HEAD^{tree}`, `make ci` left the host's checkout clean, and your worktree is still clean and on that tree when the run ends; otherwise it writes nothing. Beside the marker, `tmp/ci-passed-tree-<tree>.provenance` records the host, its OS and architecture, the start and end times and the log path, and the pre-push hook prints `make ci passed on <host> (<os>/<arch>)` from it. A marker from a local `make ci` has no provenance file and works exactly as before. + ### Test Structure | Category | Location | Docker? | Command | @@ -533,7 +549,7 @@ Run `make help` to see all targets. Key ones: | `make tidy` | Verify `go.mod`/`go.sum` are tidy (run `make fix` to apply) | | `make lint` | Run linters across Go (`golangci-lint`) + TS (Biome) + Markdown/MDX (markdownlint) + prose (misspell) | | `make vulncheck` | Run `govulncheck -scan package` (`V=1`: the default symbol-level scan, with example traces) | -| `make verify` | Repo-wide static checks: Go (tidy + fmt + vulncheck + lint) + TS (Biome + `tsc` typecheck) + Markdown/MDX (markdownlint + rule fixtures) + prose (misspell) + shell (shellcheck) + workflows (actionlint) + path-classifier fixtures + release-channel fixtures + go-toolchain fixtures + Dockerfile Go-version check + tagged-test selector and integration-parts fixtures + PR build-cache prune fixtures + review-gate hook tests + docs type-check (`astro check` — not a full build, so link validation is left to `make build-docs`, which `make ci` runs); a bare `make verify` runs these in parallel | +| `make verify` | Repo-wide static checks: Go (tidy + fmt + vulncheck + lint) + TS (Biome + `tsc` typecheck) + Markdown/MDX (markdownlint + rule fixtures) + prose (misspell) + shell (shellcheck) + workflows (actionlint) + path-classifier fixtures + release-channel fixtures + go-toolchain fixtures + Dockerfile Go-version check + tagged-test selector and integration-parts fixtures + PR build-cache prune fixtures + review-gate hook tests + git-hook tests (`.githooks/`) + Claude Code hook-command tests (`.claude/settings.json`) + `make ci-remote` tests + docs type-check (`astro check` — not a full build, so link validation is left to `make build-docs`, which `make ci` runs); a bare `make verify` runs these in parallel | | `make fix` | Auto-fixes across Go (`tidy` + `gofumpt` + `goimports` + `lint --fix`), TS (Biome `--write`), Markdown (markdownlint `--fix`), MDX (`fix-mdx-fences` only — the generic fixers never run over `.mdx`), and docs-prose spelling (misspell, both) | | **Build** | | | `make build` | Compile `wavehouse` → `bin/wavehouse` (debug symbols kept) | @@ -549,6 +565,7 @@ Run `make help` to see all targets. Key ones: | `make test-e2e` | E2E SDK suite against `bin/wavehouse-cov` + coverage gate | | `make test-all` | All four suites sequentially + merged coverage gate | | `make ci` | Full pipeline: parallel `verify` + builds + unit/SDK tests, then integration + E2E + cov | +| `make ci-remote HOST=<host>` | `make ci` for `HEAD` on an ssh host or `docker:<container>`; writes the same marker if it passes there for this exact tree ([details](#running-make-ci-on-another-machine)) | | **Release** (see [Cutting a release](#cutting-a-release)) | | | `make release-server VERSION=X.Y.Z` | Tag a server release — binaries + container image | | `make release-sdk-ts VERSION=X.Y.Z` | Tag a TypeScript SDK release — npm | diff --git a/scripts/ci-marker.sh b/scripts/ci-marker.sh index 8b59e8c0..46b9c81b 100755 --- a/scripts/ci-marker.sh +++ b/scripts/ci-marker.sh @@ -3,7 +3,9 @@ # tmp/ci-passed-tree-<TREE> — full `make ci` passed # tmp/verify-passed-tree-<TREE> — just `make verify` passed (subset of ci) # `make ci` writes both (it runs verify); `make verify` writes only the verify -# marker. Pre-commit skips re-running verify when the marker is current; +# marker; `make ci-remote` (scripts/ci/remote-ci.sh) writes both, with a +# provenance file beside the ci marker, after `make ci` passes on another +# machine. Pre-commit skips re-running verify when the marker is current; # pre-push consults the ci marker. Skipped on CI runners. set -euo pipefail diff --git a/scripts/ci/remote-ci.sh b/scripts/ci/remote-ci.sh new file mode 100755 index 00000000..cdb08517 --- /dev/null +++ b/scripts/ci/remote-ci.sh @@ -0,0 +1,160 @@ +#!/usr/bin/env bash +# Run `make ci` for HEAD on another machine and, when it passes there for +# exactly this tree, write the markers a local `make ci` would +# (tmp/ci-passed-tree-<tree> and the verify marker), plus a provenance file +# beside them that .githooks/pre-push prints. `make ci-remote` calls this. +# +# Usage: scripts/ci/remote-ci.sh <host> +# <host> an ssh destination (user@box, a ~/.ssh/config alias, ...), or +# docker:<container> for a running local container +# KEEP=1 leave the run directory on the host +# REMOTE_DIR base directory for per-run checkouts on the host; a relative +# path is under the host's home (default .cache/wavehouse-ci) +# +# The host needs bash, git, GNU make 4+, curl and jq, and what `make ci` itself +# needs: Go, Node and pnpm for `make tools`, and a Docker daemon for the +# integration and e2e suites. Over ssh the commands run in a login shell, so +# PATH comes from the host's profile; in a container it is the container's own. +# +# Nothing is pushed. HEAD travels as a git bundle on the transport's stdin and +# is checked out in a fresh directory under REMOTE_DIR, which is removed after +# the run unless KEEP=1. When this script exits, however it is stopped, the +# host stops the run within about a second. +# The markers are written only if the host exits 0, the tree it reports having +# tested is HEAD^{tree} here, and this worktree is still clean. + +set -uo pipefail +cd "$(git rev-parse --show-toplevel)" || exit 1 + +die() { + printf '🛑 ci-remote: %s\n' "$*" >&2 + exit 1 +} + +# Runs on the host. bash reads a script from a pipe one byte at a time, so +# after it reads the call below, the bundle that follows is left for `head`. +# Report lines carry this run's nonce, which make's output cannot know. +remote_main() { + set -eu + exec 2>&1 + say() { printf 'ci-remote[%s] %s\n' "$nonce" "$*"; } + # shellcheck disable=SC2329 # run by the EXIT trap + finish() { + if [ -n "${watcher:-}" ]; then kill -- "-$watcher" 2> /dev/null || true; fi + if [ "$keep" = 1 ]; then say "kept $dir"; else rm -rf "$dir"; fi + } + case $base in + /*) ;; + *) base=$HOME/${base#"~/"} ;; + esac + mkdir -p "$base" + dir=$(mktemp -d "$base/run.XXXXXX") + trap finish EXIT + say "uname $(uname -sm)" + say "dir $dir" + head -c "$size" > "$dir/head.bundle" + git clone -q --no-checkout "$dir/head.bundle" "$dir/src" + cd "$dir/src" + git -c advice.detachedHead=false checkout -q --detach "$commit" + say "tree $(git rev-parse 'HEAD^{tree}')" + if [ -n "$no_color" ]; then export NO_COLOR="$no_color"; fi + # Stdin stays open until the local side exits. When it closes first, nobody + # is waiting for the run, so stop it: job control gives it a process group + # of its own to signal. + set -m + { make tools && make ci; } < /dev/null & + run=$! + { cat > /dev/null; kill -TERM -- "-$run"; } > /dev/null 2>&1 & + watcher=$! + wait "$run" + # A local `make ci` keys its marker on the tree it leaves behind. + if [ -n "$(git status --porcelain --untracked-files=normal)" ]; then + echo "make ci changed the checkout:" + git status --short --untracked-files=normal + exit 1 + fi +} + +host=${1:-} +case $host in + docker:*) target=${host#docker:} ;; + *) target=$host ;; +esac +case $target in + "") die "usage: make ci-remote HOST=<ssh destination>|docker:<container>" ;; + -* | *[[:space:][:cntrl:]]*) die "not a usable host: '$host'" ;; +esac +case $host in + docker:*) transport=(docker exec -i "$target" bash -s) ;; + *) transport=(ssh -T "$target" bash -l -s) ;; +esac + +clean() { + local s + s=$(git status --porcelain --untracked-files=normal) && [ -z "$s" ] +} +clean || die "the worktree has uncommitted or untracked changes, and the host only gets HEAD. Commit them first." + +commit=$(git rev-parse --verify HEAD) || die "no HEAD commit" +tree=$(git rev-parse "$commit^{tree}") || die "can't resolve the tree of ${commit:0:8}" +keep=0 +[ "${KEEP:-}" = 1 ] && keep=1 +base=${REMOTE_DIR:-.cache/wavehouse-ci} +nonce=$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n') +[ -n "$nonce" ] || die "can't read /dev/urandom" + +bundle=$(mktemp) || die "can't create a temporary file" +git bundle create -q "$bundle" HEAD || { rm -f "$bundle"; die "can't bundle HEAD"; } +size=$(($(wc -c < "$bundle"))) +# Read through fd 3 from here on, so the file is gone however the script ends. +exec 3< "$bundle" +rm -f "$bundle" + +mkdir -p tmp +log=tmp/ci-remote-$tree-$nonce.log +printf '==> make ci on %s for %s (tree %s), log in %s\n' "$host" "${commit:0:8}" "${tree:0:8}" "$log" >&2 +started=$(date -u +%Y-%m-%dT%H:%M:%SZ) +{ + printf 'commit=%q base=%q keep=%q nonce=%q no_color=%q size=%q\n' "$commit" "$base" "$keep" "$nonce" "${NO_COLOR:-}" "$size" + declare -f remote_main + echo 'remote_main; exit' + cat <&3 + # Hold the host's stdin open while this script runs ($$ is still its PID in + # this subshell): a signal to the script alone leaves the transport running. + while kill -0 "$$" 2> /dev/null && echo; do sleep 1; done 2> /dev/null +} | "${transport[@]}" 2>&1 | tee "$log" +status=("${PIPESTATUS[@]}") +finished=$(date -u +%Y-%m-%dT%H:%M:%SZ) + +report() { + awk -v p="ci-remote[$nonce] $1 " 'index($0, p) == 1 { print substr($0, length(p) + 1); exit }' "$log" +} +uname_sm=$(report uname) +remote_dir=$(report dir) +tested=$(report tree) + +[ "${status[1]}" = 0 ] || die "make ci failed on $host (exit ${status[1]}); no marker written. Log: $log" +[ "${status[2]}" = 0 ] || die "can't write $log; no marker written." +[ "$tested" = "$tree" ] || die "$host reported testing tree '${tested:0:8}', not ${tree:0:8}; no marker written." +now=$(git rev-parse "HEAD^{tree}") || die "can't resolve HEAD" +[ "$now" = "$tree" ] || die "HEAD moved to tree ${now:0:8} during the run, which tested ${tree:0:8}; no marker written. Rerun." +clean || die "the worktree changed during the run; no marker written. Commit, then rerun." + +marker=$(scripts/ci-marker.sh path-for-commit "$commit") || die "can't resolve the marker path" +verify_marker=$(scripts/ci-marker.sh verify-path-for-commit "$commit") || die "can't resolve the marker path" +prov=$marker.provenance +{ + echo "host=$host" + echo "os=${uname_sm%% *}" + echo "arch=${uname_sm##* }" + echo "commit=$commit" + echo "tree=$tree" + echo "started=$started" + echo "finished=$finished" + echo "log=$log" + echo "remote_dir=$remote_dir" + echo "kept=$keep" +} > "$prov.tmp" || die "can't write $prov" +mv "$prov.tmp" "$prov" || die "can't write $prov" +touch "$marker" "$verify_marker" || die "can't write $marker" +printf '✔ ci-remote: make ci passed on %s (%s) for %s; wrote %s\n' "$host" "$uname_sm" "${commit:0:8}" "$marker" >&2 diff --git a/scripts/ci/remote-ci.test.sh b/scripts/ci/remote-ci.test.sh new file mode 100755 index 00000000..aed0f98f --- /dev/null +++ b/scripts/ci/remote-ci.test.sh @@ -0,0 +1,341 @@ +#!/usr/bin/env bash +# Behavioral test for scripts/ci/remote-ci.sh (`make ci-remote`) and the +# provenance line .githooks/pre-push prints for its marker. Runs the script in +# a scratch repository with stand-in `ssh` and `docker` commands on PATH: they +# record how they were called, then run the host side for real in a scratch +# home against a stub `make` and `uname`, so no host, network or Docker is +# needed. Markers come only from the script under test. Run by `make verify` +# (target: test-remote-ci). Needs git. + +set -uo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/../.." || exit 1 # repo root (scripts/ci/../..) +fails=0 + +scratch=$(mktemp -d) || exit 1 +trap 'rm -rf "$scratch"' EXIT +scratch=$(cd "$scratch" && pwd -P) + +# Run from a git hook (pre-commit runs make verify), GIT_DIR and friends point +# at this checkout, so a `git init` here would re-initialize it. Clear them, +# refuse to run unless git sees the scratch directory as outside any +# repository, and name every repository with -C. +# shellcheck disable=SC2046 # one variable name per word +unset $(git -C "$scratch" rev-parse --local-env-vars) +# Hiding untracked files from `git status` leaves the scripts' own +# --untracked-files=normal as the only thing that finds them. +printf '[status]\n\tshowUntrackedFiles = no\n' > "$scratch/gitconfig" +export GIT_CONFIG_GLOBAL=$scratch/gitconfig GIT_CONFIG_NOSYSTEM=1 +if git -C "$scratch" rev-parse --git-dir >/dev/null 2>&1; then + echo "remote-ci test: git resolves $scratch to an existing repository; refusing to run." >&2 + exit 1 +fi +export GIT_AUTHOR_NAME=t GIT_AUTHOR_EMAIL=t@example.com GIT_COMMITTER_NAME=t GIT_COMMITTER_EMAIL=t@example.com + +repo=$scratch/repo +git -C "$scratch" init -q -b main "$repo" +if [ "$(git -C "$repo" rev-parse --absolute-git-dir 2>/dev/null)" != "$repo/.git" ]; then + echo "remote-ci test: git did not create $repo/.git; refusing to go on." >&2 + exit 1 +fi +mkdir -p "$repo/scripts/ci" "$repo/.githooks" +cp scripts/ci-marker.sh scripts/classify-paths.sh "$repo/scripts/" +cp scripts/ci/remote-ci.sh "$repo/scripts/ci/" +cp .githooks/pre-push "$repo/.githooks/" +echo 'tmp/' > "$repo/.gitignore" +# A code repository has a Makefile; the pre-push hook passes a commit without one. +printf 'ci:\n' > "$repo/Makefile" +git -C "$repo" add -A +git -C "$repo" commit -q -m base +git -C "$repo" update-ref refs/remotes/origin/main HEAD +echo 'package main' > "$repo/main.go" +git -C "$repo" add main.go +git -C "$repo" commit -q -m code + +# ── Stand-ins ── +# bin/ssh, bin/docker record their arguments, then run the host side in +# a process group of its own, as sshd would +# host-bin/make records each call with the tree of the checkout it +# runs in; `make ci` exits with the code in make-exit +# after running the during-ci script, if there is one +# host-bin/uname a Linux amd64 host +# host-bin/git git, except that a checkout lands on the commit +# named in wrong-commit, if there is one +mkdir -p "$scratch/bin" "$scratch/host-bin" +for cmd in ssh docker; do + cat > "$scratch/bin/$cmd" << EOF +#!/usr/bin/env bash +echo "$cmd \$*" >> "$scratch/transport.log" +cd "$scratch/home" || exit 1 +set -m +HOME="$scratch/home" PATH="$scratch/host-bin:\$PATH" bash -s & +wait \$! +EOF +done +cat > "$scratch/host-bin/make" << EOF +#!/usr/bin/env bash +echo "make \$* in \$(git rev-parse 'HEAD^{tree}')" >> "$scratch/make.log" +[ "\$1" = ci ] || exit 0 +echo "stub make ci output" +[ ! -f "$scratch/during-ci" ] || bash "$scratch/during-ci" +exit "\$(cat "$scratch/make-exit")" +EOF +printf '#!/bin/sh\necho "Linux x86_64"\n' > "$scratch/host-bin/uname" +cat > "$scratch/host-bin/git" << EOF +#!/usr/bin/env bash +if [ -f "$scratch/wrong-commit" ] && [[ " \$* " == *" checkout "* ]]; then + set -- "\${@:1:\$#-1}" "\$(cat "$scratch/wrong-commit")" +fi +exec "$(command -v git)" "\$@" +EOF +chmod +x "$scratch/bin/"* "$scratch/host-bin/"* + +ok() { printf ' ok %s\n' "$1"; } +fail() { + printf ' FAIL %s\n' "$1" >&2 + [ -z "${2:-}" ] || printf '%s\n' "$2" | sed 's/^/ /' >&2 + fails=$((fails + 1)) +} + +# reset: a clean slate for the next case. +reset() { + rm -rf "$repo/tmp" "${scratch:?}/home" "$scratch/runs" "$scratch/during-ci" "$scratch/wrong-commit" + mkdir -p "$scratch/home" + : > "$scratch/transport.log" + : > "$scratch/make.log" + echo 0 > "$scratch/make-exit" +} + +# start <host> [VAR=value…]: the script under test, in the scratch repository, +# in the background; $! is the script's own PID. +start() { + local host=$1 + shift + (cd "$repo" && exec env PATH="$scratch/bin:$PATH" "$@" scripts/ci/remote-ci.sh "$host") > "$scratch/out" 2>&1 & +} + +# run <host> [VAR=value…]: start, then wait. A run that hangs is stopped after +# 60s, so a regression fails here instead of stalling `make verify`. +run() { + local pid dog rc + start "$@" + pid=$! + (sleep 60 && kill -TERM "$pid") > /dev/null 2>&1 & + dog=$! + wait "$pid" + rc=$? + kill "$dog" 2> /dev/null + return "$rc" +} + +tree() { git -C "$repo" rev-parse 'HEAD^{tree}'; } +marker() { echo "$repo/tmp/ci-passed-tree-$1"; } +prov() { sed -n "s/^$1=//p" "$(marker "$2").provenance"; } +leftover_runs() { find "$scratch/home" "$scratch/runs" -name 'run.*' -prune 2>/dev/null; } + +expect_marker() { # <name> <tree> + if [ -f "$(marker "$2")" ] && [ -f "$repo/tmp/verify-passed-tree-$2" ] && [ -f "$(marker "$2").provenance" ]; then + ok "$1" + else + fail "$1: no marker and provenance for tree ${2:0:8}" "$(cat "$scratch/out")" + fi +} +expect_nothing() { # <name> <stderr substring> <tree…> + local name=$1 want=$2 t + shift 2 + for t in "$@"; do + if [ -e "$(marker "$t")" ] || [ -e "$(marker "$t").provenance" ] || [ -e "$repo/tmp/verify-passed-tree-$t" ]; then + fail "$name: wrote a marker for tree ${t:0:8}" + return + fi + done + if ! grep -qF -- "$want" "$scratch/out"; then + fail "$name: output lacks '$want'" "$(cat "$scratch/out")" + else + ok "$name" + fi +} +expect_eq() { # <name> <got> <want> + if [ "$2" = "$3" ]; then ok "$1"; else fail "$1" "got: $2"$'\n'"want: $3"; fi +} + +# ── A passing run over ssh ── +reset +T=$(tree) +C=$(git -C "$repo" rev-parse HEAD) +if run devbox; then + expect_marker "a passing run writes the markers and provenance" "$T" +else + fail "a passing run exits 0" "$(cat "$scratch/out")" +fi +expect_eq "…over ssh, in a login shell" "$(cat "$scratch/transport.log")" "ssh -T devbox bash -l -s" +expect_eq "…running make tools, then make ci, in a checkout of HEAD's tree" "$(cat "$scratch/make.log")" "make tools in $T"$'\n'"make ci in $T" +expect_eq "…recording the host" "$(prov host "$T")" devbox +expect_eq "…its OS and architecture as the host reports them" "$(prov os "$T")/$(prov arch "$T")" "Linux/x86_64" +expect_eq "…the commit and tree" "$(prov commit "$T") $(prov tree "$T")" "$C $T" +iso='^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$' +if [[ $(prov started "$T") =~ $iso ]] && [[ $(prov finished "$T") =~ $iso ]]; then + ok "…the start and end times" +else + fail "…the start and end times" "$(cat "$(marker "$T").provenance")" +fi +log=$repo/$(prov log "$T") +if [[ $(head -n 1 "$log") == "ci-remote["*"] uname Linux x86_64" ]] && grep -qx "stub make ci output" "$log"; then + ok "…and a local copy of the log, which opens with the host's uname" +else + fail "…and a local copy of the log, which opens with the host's uname" "$(cat "$log")" +fi +expect_eq "…and removes the run directory from the host" "$(leftover_runs)" "" + +# ── KEEP and REMOTE_DIR ── +reset +if run devbox KEEP=1 REMOTE_DIR="$scratch/runs"; then + dir=$(prov remote_dir "$T") + if [ "$(prov kept "$T")" = 1 ] && [ "${dir#"$scratch/runs/run."}" != "$dir" ] && [ -d "$dir/src" ]; then + ok "KEEP=1 leaves the run directory under REMOTE_DIR" + else + fail "KEEP=1 leaves the run directory under REMOTE_DIR" "$(cat "$(marker "$T").provenance")" + fi +else + fail "KEEP=1 run exits 0" "$(cat "$scratch/out")" +fi + +# ── Failures write nothing ── +reset +echo 2 > "$scratch/make-exit" +if run devbox; then fail "a failing make ci exits non-zero"; fi +expect_nothing "a failing make ci writes nothing" "make ci failed on devbox (exit 2)" "$T" +expect_eq "…and still cleans up the host" "$(leftover_runs)" "" + +reset +cat > "$scratch/during-ci" << EOF +echo change > "$repo/main.go" +git -C "$repo" commit -qam 'commit during the run' +EOF +if run devbox; then fail "HEAD moving during the run exits non-zero"; fi +expect_nothing "a tree mismatch (HEAD moved during the run) writes nothing" "HEAD moved to tree" "$T" "$(tree)" +git -C "$repo" reset -q --hard "$C" + +reset +git -C "$repo" rev-parse HEAD~1 > "$scratch/wrong-commit" +if run devbox; then fail "a host that tested another tree exits non-zero"; fi +expect_nothing "a tree mismatch (the host tested another tree) writes nothing" "reported testing tree" "$T" + +reset +git -C "$repo" rev-parse HEAD~1 > "$scratch/wrong-commit" +echo "echo 'ci-remote[000000000000] tree $T'" > "$scratch/during-ci" +if run devbox; then fail "a report line forged by make's output exits non-zero"; fi +expect_nothing "a report line forged by make's output doesn't count" "reported testing tree" "$T" + +reset +echo "touch '$repo/new-file'" > "$scratch/during-ci" +if run devbox; then fail "a worktree dirtied during the run exits non-zero"; fi +expect_nothing "a worktree dirtied during the run writes nothing" "the worktree changed during the run" "$T" +rm -f "$repo/new-file" + +reset +echo "touch generated-file" > "$scratch/during-ci" # cwd: the host's checkout +if run devbox; then fail "make ci dirtying the host checkout exits non-zero"; fi +expect_nothing "make ci leaving the host checkout dirty writes nothing" "make ci changed the checkout" "$T" + +# ── Interrupted: the host stops the run and cleans up ── +# The host's make ci records its PID, then waits. +hold_ci() { echo "echo \$\$ > '$scratch/ci.pid'; exec sleep 60" > "$scratch/during-ci"; } +wait_for_ci() { for _ in $(seq 100); do [ -s "$scratch/ci.pid" ] && break; sleep 0.1; done; } +expect_host_stopped() { # <name> + local pid + pid=$(cat "$scratch/ci.pid" 2> /dev/null) + for _ in $(seq 100); do + [ -n "$pid" ] && [ -z "$(leftover_runs)" ] && ! kill -0 "$pid" 2> /dev/null && break + sleep 0.1 + done + if [ -n "$pid" ] && [ -z "$(leftover_runs)" ] && ! kill -0 "$pid" 2> /dev/null; then + ok "$1" + else + fail "$1" "${pid:-make ci never started}; left: $(leftover_runs)" + [ -z "$pid" ] || kill "$pid" 2> /dev/null + fi + rm -f "$scratch/ci.pid" +} + +reset +hold_ci +set -m +run devbox & +local_side=$! +set +m +wait_for_ci +kill -TERM -- "-$local_side" 2> /dev/null # what Ctrl-C does: the whole local process group +wait "$local_side" 2> /dev/null +expect_host_stopped "stopping the local process group stops make ci on the host and removes its run directory" +expect_nothing "…and writes nothing" "make ci on devbox" "$T" + +reset +hold_ci +start devbox +script=$! +wait_for_ci +kill -TERM "$script" # the script alone: its pipeline is left running +wait "$script" 2> /dev/null +expect_host_stopped "signaling only the script stops make ci on the host and removes its run directory" +expect_nothing "…and writes nothing" "make ci on devbox" "$T" + +# ── Refusals: nothing reaches the host ── +reset +echo change > "$repo/main.go" +if run devbox; then fail "a modified file is refused"; fi +expect_nothing "a modified tracked file is refused" "uncommitted or untracked changes" "$T" +git -C "$repo" checkout -q -- main.go +touch "$repo/untracked" +if run devbox; then fail "an untracked file is refused"; fi +expect_nothing "an untracked file is refused" "uncommitted or untracked changes" "$T" +rm -f "$repo/untracked" +for bad in "" "docker:" "-oProxyCommand=sh" "docker:-it" "dev box"; do + if run "$bad"; then fail "HOST '$bad' is refused"; fi +done +expect_eq "…and so are unusable HOST values, none of them contacting the host" "$(cat "$scratch/transport.log")" "" + +# ── A container ── +reset +if run docker:wh-dev; then + expect_marker "docker:<container> writes the markers" "$T" +else + fail "docker:<container> run exits 0" "$(cat "$scratch/out")" +fi +expect_eq "…through docker exec, in the container's own environment" "$(cat "$scratch/transport.log")" "docker exec -i wh-dev bash -s" +expect_eq "…recording the container as the host" "$(prov host "$T")" "docker:wh-dev" + +# ── Two runs of one tree keep their own logs ── +reset +run devbox +log_a=$(prov log "$T") +run docker:wh-dev +log_b=$(prov log "$T") +if [ "$log_a" != "$log_b" ] && [ -f "$repo/$log_a" ] && [ -f "$repo/$log_b" ] \ + && [ "$(head -n 1 "$repo/$log_a")" != "$(head -n 1 "$repo/$log_b")" ]; then + ok "two runs of the same tree write separate logs, and the provenance names this run's" +else + fail "two runs of the same tree write separate logs, and the provenance names this run's" "$log_a"$'\n'"$log_b" +fi + +# ── The pre-push hook names where make ci passed ── +zero=0000000000000000000000000000000000000000 +push() { (cd "$repo" && echo "refs/heads/main $C refs/heads/main $zero" | .githooks/pre-push origin url) > "$scratch/out" 2>&1; } +if push && grep -qF "make ci passed on docker:wh-dev (Linux/x86_64)" "$scratch/out"; then + ok "pre-push accepts the marker and names the host" +else + fail "pre-push accepts the marker and names the host" "$(cat "$scratch/out")" +fi +rm -f "$(marker "$T").provenance" +if push && ! grep -qF "passed on" "$scratch/out"; then + ok "…and says nothing extra for a marker without provenance" +else + fail "…and says nothing extra for a marker without provenance" "$(cat "$scratch/out")" +fi +rm -f "$(marker "$T")" +if push; then fail "pre-push blocks a code push with no marker"; else ok "pre-push blocks a code push with no marker"; fi + +if [ "$fails" -gt 0 ]; then + printf '\n%d case(s) failed\n' "$fails" >&2 + exit 1 +fi +echo "remote-ci: all cases passed" diff --git a/scripts/githooks.test.sh b/scripts/githooks.test.sh new file mode 100755 index 00000000..ed763355 --- /dev/null +++ b/scripts/githooks.test.sh @@ -0,0 +1,180 @@ +#!/usr/bin/env bash +# Behavioral test for the git hooks in .githooks/. Builds a scratch repository +# whose core.hooksPath names this checkout's .githooks/ by absolute path, the +# setup that runs them in every linked worktree, and commits and pushes from +# worktrees with and without code: a worktree whose branch has no Makefile must +# commit and push its own branch (#678), while code still needs its markers and +# a missing scripts/ci-marker.sh is reported, not swallowed. The repository's +# Makefile is a stub whose `verify` only says it ran. Markers come only from +# scripts/ci-marker.sh, never from the test itself. Run by `make verify` +# (target: test-githooks). Needs git and make; no network. + +set -uo pipefail + +# The git hooks run under whichever bash `#!/usr/bin/env bash` finds first. +# macOS ships bash 3.2 as /bin/bash, so when that is another bash the suite runs +# a second time with it first on PATH (review-gate.test.sh does the same). +if [ -z "${HOOK_TEST_BASH:-}" ]; then + self="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")" + HOOK_TEST_BASH=$(command -v bash) "$BASH" "$self" + rc=$? + if [ -x /bin/bash ] && ! [ /bin/bash -ef "$(command -v bash)" ]; then + shim=$(mktemp -d) && ln -s /bin/bash "$shim/bash" || exit 1 + echo "── the hooks again under /bin/bash $(/bin/bash -c 'echo "$BASH_VERSION"')" + PATH="$shim:$PATH" HOOK_TEST_BASH=/bin/bash "$BASH" "$self" || rc=1 + rm -rf "$shim" + fi + exit "$rc" +fi + +cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit 1 # repo root (scripts/..) +root=$PWD +fails=0 + +scratch=$(mktemp -d) || exit 1 +trap 'rm -rf "$scratch"' EXIT +scratch=$(cd "$scratch" && pwd -P) + +# Run from a git hook (pre-commit runs make verify), GIT_DIR and friends point +# at this checkout: a `git init` here would re-initialize it instead of creating +# the scratch repository. Clear them, then refuse to write anything unless git +# sees the scratch directory as outside any repository and puts the new +# repository exactly where it was asked to. Every git call below names its +# directory with -C. The outer make's MAKEFLAGS would hand the stub make its +# jobserver, so clear those too. +# shellcheck disable=SC2046 # one variable name per word +unset $(git -C "$scratch" rev-parse --local-env-vars) MAKEFLAGS MAKELEVEL MFLAGS +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 +if git -C "$scratch" rev-parse --git-dir >/dev/null 2>&1; then + echo "githooks test: git resolves $scratch to an existing repository; refusing to run." >&2 + exit 1 +fi +export GIT_AUTHOR_NAME=t GIT_AUTHOR_EMAIL=t@example.com GIT_COMMITTER_NAME=t GIT_COMMITTER_EMAIL=t@example.com + +repo=$scratch/repo +git -C "$scratch" init -q -b main "$repo" +if [ "$(git -C "$repo" rev-parse --absolute-git-dir 2>/dev/null)" != "$repo/.git" ]; then + echo "githooks test: git did not create $repo/.git; refusing to go on." >&2 + exit 1 +fi + +ok() { printf ' ok %s\n' "$1"; } +fail() { + printf ' FAIL %s\n' "$1" >&2 + [ -n "${2:-}" ] && printf '%s\n' "$2" | sed 's/^/ /' >&2 + fails=$((fails + 1)) +} + +# expect <name> <want-exit> <pattern>... -- <dir> <git args>: run git in <dir> +# and check its exit code and combined output (a hook's output reaches git's +# stderr). A pattern is a fixed string the output must contain, or, led by +# `!`, one it must not. +expect() { + local name=$1 want=$2 got out p bad="" + shift 2 + local pats=() + while [ "$1" != -- ]; do pats+=("$1"); shift; done + shift + local dir=$1 + shift + out=$(git -C "$dir" "$@" 2>&1) + got=$? + [ "$got" = "$want" ] || bad="exit $got, want $want" + for p in "${pats[@]}"; do + case $p in + !*) case $out in *"${p#!}"*) bad="${bad:+$bad; }output has '${p#!}'" ;; esac ;; + *) case $out in *"$p"*) ;; *) bad="${bad:+$bad; }output lacks '$p'" ;; esac ;; + esac + done + if [ -z "$bad" ]; then ok "$name"; else fail "$name: $bad" "$out"; fi +} + +# in_wt <dir> <command>…: run a command from the top of the scratch worktree <dir>, +# or stop if git doesn't see <dir> as one: scripts/ci-marker.sh writes its +# markers into whichever worktree it runs in. +in_wt() { + local dir=$1 + shift + if [ "$(git -C "$dir" rev-parse --show-toplevel 2>/dev/null)" != "$dir" ]; then + echo "githooks test: $dir is not a scratch worktree; refusing to go on." >&2 + exit 1 + fi + (cd "$dir" && "$@") +} + +# ── Scratch repository ── +# repo/ — main worktree on main: a stub Makefile and the real marker and +# classifier scripts +# code/ — linked worktree on feat +# state/ — linked worktree on the orphan branch `state`: three text files, +# no Makefile, no scripts +mkdir -p "$repo/scripts" +cp scripts/ci-marker.sh scripts/classify-paths.sh "$repo/scripts/" +# shellcheck disable=SC2016 # make's $$, not the shell's +printf 'verify:\n\t@echo "stub verify ran"\n\t@test -z "$$STUB_VERIFY_FAIL"\n' > "$repo/Makefile" +printf 'tmp/\n' > "$repo/.gitignore" +git -C "$repo" add -A +git -C "$repo" commit -q -m base +code=$scratch/code +state=$scratch/state +git -C "$repo" worktree add -q -b feat "$code" main +git -C "$repo" worktree add -q --detach "$state" main +git -C "$state" checkout -q --orphan state +in_wt "$state" git rm -rqf . +printf '{}\n' > "$state/state.json" +git -C "$state" add state.json +git -C "$state" commit -q -m 'state: first run' +# The remote has main alone, so every branch pushed below is new to it. +git -C "$scratch" init -q --bare remote.git +git -C "$scratch/remote.git" fetch -q "$repo" main:main +git -C "$repo" remote add origin "$scratch/remote.git" +git -C "$repo" fetch -q origin + +# Hooks from here on: this checkout's, by absolute path, in every worktree. +git -C "$repo" config core.hooksPath "$root/.githooks" + +# ── pre-commit ── +printf 'run 2\n' > "$state/runs.log" +git -C "$state" add runs.log +expect "pre-commit: a worktree with no Makefile commits" 0 "no Makefile" "!make verify" -- "$state" commit -q -m 'state: run 2' + +printf 'package x\n' > "$code/x.go" +git -C "$code" add x.go +expect "pre-commit: code with no verify marker runs make verify" 0 "stub verify ran" "!missing" "!cached" -- "$code" commit -q -m x + +printf 'package y\n' > "$code/y.go" +git -C "$code" add y.go +in_wt "$code" env -u CI scripts/ci-marker.sh write-verify +expect "pre-commit: a verify marker for the tree skips make verify" 0 "(cached)" "!stub verify ran" -- "$code" commit -q -m y + +printf 'package z\n' > "$code/z.go" +git -C "$code" add z.go +export STUB_VERIFY_FAIL=1 +expect "pre-commit: a failing make verify blocks the commit" 1 "stub verify ran" "'make verify' failed" -- "$code" commit -q -m z +unset STUB_VERIFY_FAIL +git -C "$code" commit -q -m z >/dev/null 2>&1 || fail "setup: commit z" + +printf '#!/usr/bin/env bash\necho "marker store unreadable" >&2\nexit 3\n' > "$code/scripts/ci-marker.sh" +printf 'package w\n' > "$code/w.go" +git -C "$code" add -A +expect "pre-commit: a failing marker script is reported, then make verify runs" 0 "has-verify-marker failed" "marker store unreadable" "stub verify ran" -- "$code" commit -q -m w +git -C "$code" rm -q scripts/ci-marker.sh +expect "pre-commit: a missing marker script is reported, then make verify runs" 0 "scripts/ci-marker.sh is missing" "stub verify ran" -- "$code" commit -q -m 'drop the marker script' + +# ── pre-push ── +expect "pre-push: a branch with no Makefile pushes" 0 "has no Makefile" -- "$state" push -q origin state +expect "pre-push: code pushed from a worktree without the marker script is blocked" 1 "scripts/ci-marker.sh is missing" -- "$state" push -q origin main:fresh +expect "pre-push: code pushed from a worktree whose marker script is gone is blocked" 1 "scripts/ci-marker.sh is missing" -- "$code" push -q origin feat +git -C "$repo" switch -q -c feat2 main +printf 'package v\n' > "$repo/v.go" +git -C "$repo" add v.go +git -C "$repo" commit -q -m v >/dev/null 2>&1 || fail "setup: commit v" +expect "pre-push: code with no make ci marker is blocked" 1 "'make ci' has not been run" -- "$repo" push -q origin feat2 +in_wt "$repo" env -u CI scripts/ci-marker.sh write +expect "pre-push: code with a make ci marker for its tree pushes" 0 "!pre-push:" -- "$repo" push -q origin feat2 + +if [ "$fails" -gt 0 ]; then + printf '\n%d case(s) failed\n' "$fails" >&2 + exit 1 +fi +echo "githooks: all cases passed"