Skip to content

ci: gate every plugin hooks.json on a top-level description - #3764

Merged
kyle-sexton merged 4 commits into
mainfrom
claude/hooks-json-description-ji6rl5
Sep 5, 2026
Merged

ci: gate every plugin hooks.json on a top-level description#3764
kyle-sexton merged 4 commits into
mainfrom
claude/hooks-json-description-ji6rl5

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #3752

Summary

The plugins reference gives hooks/hooks.json an optional top-level description, the one place a plugin labels its hooks as a set. The issue counted it absent in 19 of 20 plugins, but #3727 landed a description in every file after the issue was filed, so the descriptions half is already on main. What was still missing is the gate that stops the count drifting back, and this PR adds it. No plugin file changes and no version bumps.

Fix

  • scripts/check-hooks-description.sh: scans plugins/*/hooks/hooks.json, skips a plugin with no hooks, and fails a file whose top-level description is absent, not a string, blank, or multi-line. A file that does not parse fails closed. Basis and recheck trigger are in the script header.
  • scripts/check-hooks-description.test.sh: one fixture per shape (missing, blank, whitespace, number, null, multi-line, unparsable), a CRLF-terminated file, a hook-less plugin, a mixed tree naming every failing file with the summary count, and the live tree.
  • .github/workflows/ci.yml: the lint lane runs the suite, then the gate, and reports it into ci-status beside the sibling hook gates.

Verification

  • bash scripts/check-hooks-description.test.sh: PASS=12 FAIL=0. scripts/check-hooks-description.sh on the live tree: every hooks.json (20) carries a one-line description.
  • scripts/affected-tests.sh --run (the ci.yml change selects the whole scripts/*.test.sh set): 149 shell suites passed or were skipped, exit 0.
  • actionlint, scripts/check-lane-coverage.sh --check, scripts/check-discriminating-test-skips.sh, shellcheck, shfmt, typos: clean.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme


Generated by Claude Code

The plugins reference gives hooks.json an optional top-level description,
the one place a plugin labels its hooks as a set. The 2026-09-04 audit found
it absent in 20 of 20 plugins; #3727 and #3750 added it to every file, and
nothing read the field, so nothing would notice a new plugin shipping
without it or a rewrite dropping it. scripts/check-hooks-description.sh
fails a hooks.json whose description is absent, not a string, blank or
multi-line, fails closed on a file that does not parse, and skips a plugin
with no hooks. Its suite covers each shape, a CRLF checkout, and the live
tree; the lint lane runs the suite first, then the gate, and reports it into
ci-status alongside the sibling hook gates.

Closes #3752

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 5, 2026 17:42
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T17:55:51.199709Z 1c31d29 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 36s —— View job


Review complete

  • Gather context (PR diff, CLAUDE.md, AGENTS.md)
  • Run /review:code-review skip-gate checks (not draft, not trivial, no prior lane review — full review performed)
  • Review scripts/check-hooks-description.sh
  • Review scripts/check-hooks-description.test.sh
  • Review .github/workflows/ci.yml diff
  • Post findings (summary; no inline findings warranted)

No blocking findings. This is a clean, well-scoped gate addition.

What I checked

  • scripts/check-hooks-description.sh: the classify() jq program correctly fails closed — a jq runtime error (bad JSON, or a top-level value that isn't an object) yields an empty verdict, which the caller catches as "not parseable" rather than silently passing (L53-L60). The classification order (missing → notstring → multiline → blank → ok) and the CRLF-tolerance reasoning in the header comment both check out against the jq logic. The unmatched-glob guard ([[ -f "$file" ]] || continue) correctly handles the case of zero plugins/*/hooks/hooks.json matches.
  • scripts/check-hooks-description.test.sh: 12 ok() calls match the claimed PASS=12. Each fixture isolates one shape (missing/blank/whitespace/number/null/multiline/CRLF/unparsable/hook-less-plugin/mixed-tree), uses the shared scripts/lib/test-harness.sh contract correctly (sources it, ends on test_harness::report), and the live-tree assertion at the end guards against the fixture logic diverging from reality.
  • .github/workflows/ci.yml: the new steps (self-test unguarded, gate step with id: hooks_description + continue-on-error: true, outcome wired into the ci-status summary block) follow the exact same shape as the sibling gates (hook_exec_form, killswitch_hoist) immediately above them. Both new files are committed with the executable bit (100755), matching how the workflow invokes check-hooks-description.sh directly (no bash prefix).
  • Confirmed via git diff origin/main...HEAD --stat that the change is scoped to exactly the 3 files listed, no plugin hooks.json files touched, no version bumps.

Not independently verified

I could not execute shell scripts in this environment (this lane's tool grant is Bash(gh pr diff/view/comment/review:*) plus Read/Glob/Grep — no general Bash, so I could not run bash scripts/check-hooks-description.test.sh, the full test suite, actionlint, shellcheck, or shfmt). The PR body's PASS=12 FAIL=0, the 149-suite affected-tests.sh --run result, and the lint-tool-clean claims are author-claimed and unverified by me — I confirmed them only via static reading of the diff and cross-referencing the counted ok() calls, not by running the tools myself.

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 57s —— View job


Security review

  • Gather context (PR diff, files changed)
  • Review .github/workflows/ci.yml, scripts/check-hooks-description.sh, scripts/check-hooks-description.test.sh
  • Apply skip gate / report findings

No security issues found.

This PR adds a CI lint gate (scripts/check-hooks-description.sh) that scans plugins/*/hooks/hooks.json with jq and fails if the top-level description is missing, non-string, blank, or multi-line, plus its test suite (scripts/check-hooks-description.test.sh) and a corresponding lane wiring change in .github/workflows/ci.yml.

Checked and ruled out:

  • Workflow trust boundary: the new steps run on the existing lint job (no pull_request_target/workflow_run, no permissions: widening, no new secrets exposure), and don't interpolate untrusted github.* context values into a run: block.
  • Injection surfaces: check-hooks-description.sh only globs plugins/*/hooks/hooks.json and pipes file contents to jq -r with a fixed filter — no eval, no shell interpolation of file contents, no command construction from data.
  • Fail-closed behavior: an unparsable JSON file is treated as a failure (not silently skipped), which is the safe default for a gate like this.
  • Test fixtures: use mktemp -d for isolation; no path traversal or shared-state risk.

This is a self-contained CI/lint addition with no security-relevant surface — skipping deeper review per the skip gate.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Last security-reviewed head: 1c31d299d69a3506a0a8647b7722d7a8862d02a9. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c31d299d6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/check-hooks-description.sh Outdated
A hooks.json whose first document is well-formed but carries trailing
garbage made jq print a verdict and then exit non-zero; the gate read only
the word and cleared a file Claude Code cannot load. The status is now
checked separately from the output, and more than one verdict word (two
concatenated documents) is a failure too. Two fixtures pin both shapes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme
#3773 put `if: needs.changes.outputs.run_shell == 'true'` on every detector
self-test step in the lint lane, so a diff that changes no shell file skips
the proof that the detector still works while the detector itself keeps
running. This branch's new self-test pair was written before that landed and
merged in clean, leaving the one ungated self-test step in the job.

Nothing enforces the shape: check-lane-coverage.sh reads job-to-aggregate
reachability, not step conditions, so the inconsistency would have shipped
green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019DaWEB8Daq1xAXy2Xj1Pme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hooks: add the description field to the 19 plugin hooks.json files that lack it, with a gate

2 participants