diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 123c5e591e..0ff13a4f41 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -642,6 +642,21 @@ jobs: continue-on-error: true run: scripts/check-killswitch-hoist.sh + # A plugin hooks.json takes an optional top-level `description`, the one + # place a plugin labels its hooks as a set (the plugins reference, hooks + # component). The 2026-09-04 hooks-reference audit found it absent in 20 + # of 20 plugins; #3727 and #3750 added it everywhere, and nothing else + # reads the field, so nothing else would notice a new plugin shipping + # without it (#3752). Self-test first so a broken detector cannot mask a + # regression. + - name: Test the hooks-description gate + if: needs.changes.outputs.run_shell == 'true' + run: bash scripts/check-hooks-description.test.sh + - name: Check every plugin hooks.json carries a top-level description + id: hooks_description + continue-on-error: true + run: scripts/check-hooks-description.sh + # Three separate readers judge `metadata.summary`, and they can drift # apart: a malformed value reached CI twice in #3162, failing in opposite # directions, each time after clearing the local check meant to catch it. @@ -1174,6 +1189,7 @@ jobs: userconfig-argv=${{ steps.userconfig_argv.outcome }} hook-exec-form=${{ steps.hook_exec_form.outcome }} killswitch-hoist=${{ steps.killswitch_hoist.outcome }} + hooks-description=${{ steps.hooks_description.outcome }} summary-reader-parity=${{ steps.summary_reader_parity.outcome }} plugin-options-docs=${{ steps.plugin_options_docs.outcome }} silent-skips=${{ steps.silent_skips.outcome }} diff --git a/scripts/check-hooks-description.sh b/scripts/check-hooks-description.sh new file mode 100755 index 0000000000..50f78480d0 --- /dev/null +++ b/scripts/check-hooks-description.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# Gate: every plugin hooks.json carries a top-level `description`. +# +# scripts/check-hooks-description.sh fail on any plugins/*/hooks/hooks.json +# whose top-level `description` is +# absent, not a string, blank, or +# multi-line +# +# Why: the plugins reference states "Define plugin hooks in `hooks/hooks.json` +# with an optional top-level `description` field" (raw `plugins-reference.md`, +# fetched 2026-09-05), and the field is the one place a plugin can label its +# hooks as a set, distinct from the per-handler `statusMessage` shown while a +# hook runs. The 2026-09-04 hooks-reference audit found the field absent in 20 +# of 20 plugins (#3752); #3727 and #3750 added it to every file. Nothing else +# reads the field, so nothing else would notice a new plugin shipping without +# it, or a rewrite dropping it: this gate is what keeps 20 of 20 from drifting +# back to 19. +# +# The rule: `plugins/*/hooks/hooks.json` is scanned; a plugin with no such file +# has no hooks to label and is skipped. The file must parse as JSON and its +# top-level `description` must be a string with at least one non-blank +# character and no line break. Wording is not judged here; the field is a one +# sentence label, and a reviewer reads it in the diff. +# +# Basis: https://code.claude.com/docs/en/plugins-reference (the hooks component, +# "with an optional top-level `description` field"). Recheck trigger: the +# reference renames, removes, or makes the field required; either way this +# comment and the rule are re-derived from the page, not patched from memory. + +set -euo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/.." + +if ! command -v jq >/dev/null 2>&1; then + echo "check-hooks-description: jq is required but not installed" >&2 + exit 1 +fi + +errors=0 +scanned=0 + +flag() { + local file="$1" reason="$2" + echo "HOOKS DESCRIPTION: ${file}: ${reason}" >&2 + errors=$((errors + 1)) +} + +# Classify one file's top-level description. jq prints one word per JSON +# document it reads: +# missing | notstring | blank | multiline | ok +# CRLF line endings in a Git Bash checkout sit between tokens, where jq +# treats them as whitespace; a line break INSIDE the value can only be an +# escaped one the author wrote. +# +# jq's exit status is checked SEPARATELY from its output: a file whose first +# document is well-formed but which carries trailing garbage makes jq print a +# verdict for the document and then exit non-zero on the garbage, so output +# alone would clear a file Claude Code cannot load. Only a zero exit with +# exactly one verdict word counts as parsed. +CLASSIFY_PROG=' + if has("description") | not then "missing" + elif (.description | type) != "string" then "notstring" + elif (.description | test("[\n\r]")) then "multiline" + elif (.description | gsub("^[[:space:]]+|[[:space:]]+$"; "") | length) == 0 then "blank" + else "ok" end' + +for file in plugins/*/hooks/hooks.json; do + [[ -f "$file" ]] || continue + scanned=$((scanned + 1)) + rc=0 + verdict="$(jq -r "$CLASSIFY_PROG" "$file" 2>/dev/null)" || rc=$? + if ((rc != 0)) || [[ -z "$verdict" || "$verdict" == *$'\n'* ]]; then + flag "$file" "not parseable as JSON, so this gate cannot clear it" + continue + fi + case "$verdict" in + ok) ;; + missing) flag "$file" "no top-level \"description\"; add a one-sentence label for what this plugin's hooks do and on which events" ;; + notstring) flag "$file" "top-level \"description\" is not a string" ;; + blank) flag "$file" "top-level \"description\" is blank" ;; + multiline) flag "$file" "top-level \"description\" spans more than one line; keep it to one sentence" ;; + *) flag "$file" "unexpected verdict \"$verdict\"" ;; + esac +done + +if ((errors > 0)); then + echo "check-hooks-description: ${errors} problem(s) across ${scanned} hooks.json file(s)." >&2 + exit 1 +fi +echo "Every plugin hooks.json (${scanned}) carries a one-line top-level description." diff --git a/scripts/check-hooks-description.test.sh b/scripts/check-hooks-description.test.sh new file mode 100755 index 0000000000..f338f9215d --- /dev/null +++ b/scripts/check-hooks-description.test.sh @@ -0,0 +1,160 @@ +#!/usr/bin/env bash +# Tests for scripts/check-hooks-description.sh: a fixture tree per case so the +# verdict is on the shape under test and nothing else, then the live tree. +set -uo pipefail + +SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$SELF_DIR/check-hooks-description.sh" + +# shellcheck source=lib/test-harness.sh +. "$SELF_DIR/lib/test-harness.sh" + +new_fixture() { + local dir + dir="$(mktemp -d)" + mkdir -p "$dir/scripts" "$dir/plugins" + cp "$SCRIPT" "$dir/scripts/check-hooks-description.sh" + chmod +x "$dir/scripts/check-hooks-description.sh" + printf '%s' "$dir" +} + +hooks_file() { # + mkdir -p "$1/plugins/$2/hooks" + printf '%s\n' "$3" >"$1/plugins/$2/hooks/hooks.json" +} + +run_check() ( + cd "$1" && bash scripts/check-hooks-description.sh +) + +LABELED='{"description":"Formats Go source and its imports after a write or edit.","hooks":{"PostToolUse":[{"matcher":"Write|Edit","hooks":[{"type":"command","command":"x"}]}]}}' +UNLABELED='{"hooks":{"PostToolUse":[{"matcher":"Write|Edit","hooks":[{"type":"command","command":"x"}]}]}}' + +# --- the shape the gate exists to reject: no description at all -------------- +f="$(new_fixture)" +hooks_file "$f" alpha "$UNLABELED" +if out="$(run_check "$f" 2>&1)"; then + fail "a hooks.json without a description should fail, got success: $out" +else + if echo "$out" | grep -q 'HOOKS DESCRIPTION: plugins/alpha/hooks/hooks.json: no top-level "description"'; then + ok "missing description fails and names the file" + else + fail "expected the missing-description line naming plugins/alpha, got: $out" + fi +fi +rm -rf "$f" + +# --- a labeled file passes, and a plugin with no hooks dir is skipped -------- +f="$(new_fixture)" +hooks_file "$f" alpha "$LABELED" +mkdir -p "$f/plugins/beta/skills/x" +if out="$(run_check "$f" 2>&1)"; then + if echo "$out" | grep -q 'Every plugin hooks.json (1) carries'; then + ok "a labeled file passes and a hook-less plugin is not counted" + else + fail "expected the clean statement counting 1 file, got: $out" + fi +else + fail "a labeled hooks.json should pass, got: $out" +fi +rm -rf "$f" + +# --- the degenerate values a present key can still carry --------------------- +for case in \ + 'blank|{"description":"","hooks":{}}|is blank' \ + 'whitespace|{"description":" ","hooks":{}}|is blank' \ + 'number|{"description":7,"hooks":{}}|is not a string' \ + 'null|{"description":null,"hooks":{}}|is not a string' \ + 'multiline|{"description":"Formats Go.\nAnd more.","hooks":{}}|spans more than one line'; do + IFS='|' read -r name content expect <<<"$case" + f="$(new_fixture)" + hooks_file "$f" alpha "$content" + if out="$(run_check "$f" 2>&1)"; then + fail "$name description should fail, got success: $out" + else + if echo "$out" | grep -q "HOOKS DESCRIPTION: plugins/alpha/hooks/hooks.json: .*$expect"; then + ok "$name description fails with the right reason" + else + fail "$name description: expected '$expect' in the output, got: $out" + fi + fi + rm -rf "$f" +done + +# --- a CRLF checkout (Git Bash) does not read as multi-line ----------------- +f="$(new_fixture)" +mkdir -p "$f/plugins/alpha/hooks" +printf '{\r\n "description": "Formats Go source.",\r\n "hooks": {}\r\n}\r\n' >"$f/plugins/alpha/hooks/hooks.json" +if run_check "$f" >/dev/null 2>&1; then + ok "CRLF line endings between tokens are tolerated" +else + fail "a CRLF-terminated hooks.json must pass; the line breaks sit outside the value" +fi +rm -rf "$f" + +# --- every file is named, not only the first ------------------------------- +f="$(new_fixture)" +hooks_file "$f" alpha "$UNLABELED" +hooks_file "$f" beta "$LABELED" +hooks_file "$f" gamma '{"description":"","hooks":{}}' +out="$(run_check "$f" 2>&1)" +if echo "$out" | grep -q 'plugins/alpha/hooks/hooks.json' && echo "$out" | grep -q 'plugins/gamma/hooks/hooks.json' && ! echo "$out" | grep -q 'plugins/beta/'; then + ok "both failing files are named and the passing one is not" +else + fail "expected alpha and gamma flagged, beta clean, got: $out" +fi +if echo "$out" | grep -q '2 problem(s) across 3 hooks.json file(s)'; then + ok "the summary counts problems and files" +else + fail "expected '2 problem(s) across 3 hooks.json file(s)', got: $out" +fi +rm -rf "$f" + +# --- an unparsable file is a failure, never a silent clear ----------------- +f="$(new_fixture)" +hooks_file "$f" alpha '{"description":"x", "hooks": ' +if out="$(run_check "$f" 2>&1)"; then + fail "unparsable hooks.json should fail, got success: $out" +else + if echo "$out" | grep -q 'not parseable as JSON'; then + ok "unparsable hooks.json fails closed" + else + fail "expected the parse-failure line, got: $out" + fi +fi +rm -rf "$f" + +# --- a well-formed document followed by trailing garbage is still a failure -- +# jq prints a verdict for the first document and then exits non-zero on the +# garbage; the gate must read the status, not only the word (Codex, #3764). +f="$(new_fixture)" +hooks_file "$f" alpha '{"description":"Formats Go source.","hooks":{}} trailing' +if out="$(run_check "$f" 2>&1)"; then + fail "trailing garbage after a labeled document should fail, got success: $out" +else + if echo "$out" | grep -q 'not parseable as JSON'; then + ok "trailing garbage after a labeled document fails closed" + else + fail "expected the parse-failure line for trailing garbage, got: $out" + fi +fi +rm -rf "$f" + +# --- two documents in one file is not one hooks.json either ----------------- +f="$(new_fixture)" +hooks_file "$f" alpha '{"description":"One.","hooks":{}} {"description":"Two.","hooks":{}}' +if run_check "$f" >/dev/null 2>&1; then + fail "two concatenated documents should fail" +else + ok "two concatenated documents fail closed" +fi +rm -rf "$f" + +# --- the live tree ---------------------------------------------------------- +if out="$(cd "$SELF_DIR/.." && bash scripts/check-hooks-description.sh 2>&1)"; then + ok "the repository's own hooks.json files all carry a description" +else + fail "the live tree fails the gate: $out" +fi + +test_harness::report