-
Notifications
You must be signed in to change notification settings - Fork 7
OCPEDGE-2577: Add code-quality plugin with commit convention hooks #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b3cfac3
359bbf9
bbd85e1
50fbabb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "name": "code-quality", | ||
| "description": "Code convention enforcement — commit messages, attribution, and Coderabbit configuration", | ||
|
Comment on lines
+2
to
+3
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here about the name update. |
||
| "version": "1.0.0", | ||
| "author": { "name": "jeff-roche" }, | ||
| "homepage": "https://github.com/openshift-eng/edge-tooling", | ||
| "license": "Apache-2.0" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # code-quality | ||
|
|
||
| Code convention enforcement plugin for Claude Code. Validates commit messages, checks for AI attribution trailers, and verifies Coderabbit configuration. | ||
|
|
||
| ## Hooks | ||
|
|
||
| Three hooks fire automatically: | ||
|
|
||
| | Hook | Event | Behavior | | ||
| |------|-------|----------| | ||
| | `check-commit-message.sh` | PreToolUse(Bash) | **Blocks** commits that don't follow conventional commits format | | ||
| | `check-attribution.sh` | PostToolUse(Bash) | **Warns** if AI attribution trailers are missing (advisory, non-blocking) | | ||
| | `check-coderabbit-config.sh` | SessionStart | **Reports** missing or incomplete `.coderabbit.yaml` configuration | | ||
|
|
||
| ## Conventions Enforced | ||
|
|
||
| ### Commit Messages | ||
|
|
||
| Subject line must match: `type(scope): description` | ||
|
|
||
| Valid types: `feat`, `fix`, `docs`, `refactor`, `test`, `chore`, `build`, `ci`, `perf`, `style` | ||
|
|
||
| ### AI Attribution Trailers | ||
|
|
||
| Commits should include one of: | ||
| - `Co-Authored-By: ...` | ||
| - `Assisted-by: ...` | ||
| - `Generated-by: ...` | ||
|
|
||
| ### Coderabbit Configuration | ||
|
|
||
| Repos should have a `.coderabbit.yaml` with `auto_review`, `path_filters`/`path_instructions`, and `instructions` configured. | ||
|
|
||
| ## Installation | ||
|
|
||
| ```bash | ||
| /plugin marketplace add openshift-eng/edge-tooling code-quality | ||
| ``` | ||
|
|
||
| ## License | ||
|
|
||
| Apache-2.0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| { | ||
| "hooks": { | ||
| "PreToolUse": [ | ||
| { | ||
| "matcher": "Bash", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "${CLAUDE_PLUGIN_ROOT}/scripts/check-commit-message.sh", | ||
| "timeout": 2, | ||
| "statusMessage": "Validating commit message..." | ||
|
Comment on lines
+8
to
+11
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commit message checking needs to be deterministic to be valuable. The only way to do this as deterministically as possible is to have a pre-commit hook at the git level. The Claude hooks also don't work in a multi-repo development model which is table stakes for many workstreams. |
||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "PostToolUse": [ | ||
| { | ||
| "matcher": "Bash", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "${CLAUDE_PLUGIN_ROOT}/scripts/check-attribution.sh", | ||
| "timeout": 3, | ||
| "statusMessage": "Checking attribution..." | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "SessionStart": [ | ||
| { | ||
| "matcher": "startup", | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "${CLAUDE_PLUGIN_ROOT}/scripts/check-coderabbit-config.sh", | ||
| "timeout": 5, | ||
| "statusMessage": "Checking Coderabbit config..." | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| #!/usr/bin/bash | ||
| # PostToolUse(Bash) hook — warns if AI attribution trailers are missing after commits | ||
| set -euo pipefail | ||
|
|
||
| if ! command -v jq &>/dev/null; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| INPUT=$(cat) | ||
| COMMAND=$(echo "$INPUT" | jq -r '.toolInput.command // empty') | ||
|
|
||
| # Fast path: not a commit command | ||
| if [[ "$COMMAND" != *"git commit"* ]]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Get the last commit message | ||
| COMMIT_MSG=$(git log -1 --format=%B 2>/dev/null || true) | ||
|
|
||
| if [ -z "$COMMIT_MSG" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Check for attribution trailers (case-insensitive) | ||
| if echo "$COMMIT_MSG" | grep -qi 'Co-Authored-By:'; then | ||
| exit 0 | ||
| fi | ||
| if echo "$COMMIT_MSG" | grep -qi 'Assisted-by:'; then | ||
| exit 0 | ||
| fi | ||
| if echo "$COMMIT_MSG" | grep -qi 'Generated-by:'; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # No attribution found — emit advisory context | ||
| cat <<'EOF' | ||
| { | ||
| "hookSpecificOutput": { | ||
| "hookEventName": "PostToolUse", | ||
| "additionalContext": "AI ATTRIBUTION MISSING: The last commit has no AI attribution trailer. Consider amending with one of:\n Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>\n Assisted-by: Claude Code\n Generated-by: Claude Code" | ||
| } | ||
| } | ||
| EOF | ||
|
|
||
| exit 0 | ||
|
Comment on lines
+1
to
+45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enforcing at this level is bound to create more noise than value. This is an opinionated style and it creates noise where commits are not AI generated which is still a valid case. To be frank, the guideline around this is clear so I get the motivation but the impact is very low to the point that I wouldn't want to have enforcement of this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Completely fair! I think none of this is 100% required from the perspective of our repo. This one is an easy one to cut |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| #!/usr/bin/bash | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be dropped. This isn't worth the tokens to enforce. Especially in the case of |
||
| # SessionStart hook — validates .coderabbit.yaml presence and structure | ||
| set -euo pipefail | ||
|
|
||
| if ! command -v jq &>/dev/null; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| INPUT=$(cat) | ||
| CWD=$(echo "$INPUT" | jq -r '.cwd // empty') | ||
|
|
||
| if [ -z "$CWD" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [ ! -d "$CWD/.git" ] && ! git -C "$CWD" rev-parse --git-dir &>/dev/null; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| CONFIG="$CWD/.coderabbit.yaml" | ||
|
|
||
| # Check if config exists | ||
| if [ ! -f "$CONFIG" ]; then | ||
| cat <<'EOF' | ||
| { | ||
| "hookSpecificOutput": { | ||
| "hookEventName": "SessionStart", | ||
| "additionalContext": "CODERABBIT CONFIG MISSING: No .coderabbit.yaml found. AI code review is not configured for this repo. Consider adding one with auto_review enabled, path_filters, and project-specific instructions." | ||
| } | ||
| } | ||
| EOF | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Validate config structure | ||
| MISSING=() | ||
|
|
||
| if ! grep -qE '^[[:space:]]*auto_review[[:space:]]*:' "$CONFIG"; then | ||
| MISSING+=("auto_review") | ||
| fi | ||
|
|
||
| if ! grep -qE '^[[:space:]]*(path_filters|path_instructions)[[:space:]]*:' "$CONFIG"; then | ||
| MISSING+=("path_filters or path_instructions") | ||
| fi | ||
|
|
||
| if ! grep -qE '^[[:space:]]*instructions[[:space:]]*:' "$CONFIG"; then | ||
| MISSING+=("instructions") | ||
| fi | ||
|
|
||
| # If anything missing, report findings | ||
| if [ ${#MISSING[@]} -gt 0 ]; then | ||
| MISSING_LIST=$(printf '%s, ' "${MISSING[@]}" | sed 's/, $//') | ||
| MISSING_ESCAPED=$(printf '%s' "$MISSING_LIST" | sed 's/"/\\"/g') | ||
|
|
||
| cat <<EOF | ||
| { | ||
| "hookSpecificOutput": { | ||
| "hookEventName": "SessionStart", | ||
| "additionalContext": "CODERABBIT CONFIG INCOMPLETE: .coderabbit.yaml exists but is missing: ${MISSING_ESCAPED}. Consider adding these for comprehensive AI code review coverage." | ||
| } | ||
| } | ||
| EOF | ||
| fi | ||
|
|
||
| exit 0 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| #!/usr/bin/bash | ||
| # PreToolUse(Bash) hook — validates conventional commits format on git commit commands | ||
| set -euo pipefail | ||
|
|
||
| if ! command -v jq &>/dev/null; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| INPUT=$(cat) | ||
| COMMAND=$(echo "$INPUT" | jq -r '.toolInput.command // empty') | ||
|
|
||
| # Fast path: not a commit command | ||
| if [[ "$COMMAND" != *"git commit"* ]]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Extract commit message subject line | ||
| SUBJECT="" | ||
|
|
||
| # Heredoc pattern: git commit -m "$(cat <<'EOF' ... EOF )" | ||
| if [[ "$COMMAND" =~ cat\ \<\<[\']?EOF ]]; then | ||
| # Extract first non-empty line after the heredoc delimiter | ||
| SUBJECT=$(echo "$COMMAND" | sed -n '/<<.*EOF/,/^EOF/{/<<.*EOF/d;/^EOF/d;/^[[:space:]]*$/d;p;}' | head -1 | sed 's/^[[:space:]]*//') | ||
| # Standard -m pattern with double or single quotes | ||
| elif [[ "$COMMAND" =~ git\ commit.*-m\ \" ]]; then | ||
| SUBJECT=$(echo "$COMMAND" | sed -n 's/.*git commit[^"]*-m "\([^"]*\)".*/\1/p' | head -1) | ||
| elif [[ "$COMMAND" =~ git\ commit.*-m\ \' ]]; then | ||
| SUBJECT=$(echo "$COMMAND" | sed -n "s/.*git commit[^']*-m '\\([^']*\\)'.*/\\1/p" | head -1) | ||
| fi | ||
|
|
||
| # If we couldn't extract a message, fail open | ||
| if [ -z "$SUBJECT" ]; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # For multi-line messages, take only the first line (subject) | ||
| SUBJECT=$(echo "$SUBJECT" | head -1) | ||
|
|
||
| # Validate against conventional commits pattern | ||
| if echo "$SUBJECT" | grep -qE '^(feat|fix|docs|refactor|test|chore|build|ci|perf|style)(\(.+\))?: .+'; then | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Invalid format | ||
| cat >&2 <<'MSG' | ||
| Commit message does not follow conventional commits format. | ||
| Expected: type(scope): description | ||
| Types: feat, fix, docs, refactor, test, chore, build, ci, perf, style | ||
| Examples: | ||
| feat(api): add health endpoint | ||
| fix(deploy): correct subnet CIDR validation | ||
| docs: update contributing guide | ||
| MSG | ||
| exit 1 | ||
|
Comment on lines
+1
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commit message enforcement needs to happen at the git level to be deterministic. This hook would get skipped in a multi-repo setup. I'd also say that this style is informal and not something we'd want to enforce firmly. The |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This plugin is trying to enforce an opinionated style as opposed to code quality which is something objectively measurable. This needs to be renamed to reflect that.