diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 8f5649186..ea4e007c7 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -12,7 +12,7 @@ "name": "git", "source": "./plugins/git", "description": "Git Plugin", - "version": "0.0.6", + "version": "0.0.7", "category": "tooling", "keywords": [ "git", @@ -116,7 +116,7 @@ "name": "utils", "source": "./plugins/utils", "description": "A generic utilities plugin serving as a catch-all for various helper commands", - "version": "0.0.11", + "version": "0.0.12", "category": "tooling", "keywords": [ "utilities", @@ -396,7 +396,7 @@ "name": "openshift-developer", "source": "./plugins/openshift-developer", "description": "Bundle of curated plugins, skills, and MCP servers useful to any OpenShift engineer", - "version": "1.0.0", + "version": "1.1.0", "category": "bundle", "keywords": [ "openshift", diff --git a/.skillsaw.yaml b/.skillsaw.yaml index c32599594..66a8e5982 100644 --- a/.skillsaw.yaml +++ b/.skillsaw.yaml @@ -15,6 +15,7 @@ rules: - "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/gofmt.sh" - "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/notify.sh '🔔 Claude Code' 'Claude needs your input'" - "bash ${CLAUDE_PLUGIN_ROOT}/hooks/scripts/notify.sh '✅ Claude Code' 'Claude finished your task'" + - "${CLAUDE_PLUGIN_ROOT}/scripts/ensure-precommit.sh" settings-dangerous: enabled: true severity: error diff --git a/docs/index.html b/docs/index.html index 8163945a2..bca7386de 100644 --- a/docs/index.html +++ b/docs/index.html @@ -1134,7 +1134,7 @@

ai-helpers

{ "name": "git", "description": "Git workflow automation and utilities", - "version": "0.0.6", + "version": "0.0.7", "has_readme": true, "commands": [ { @@ -1219,6 +1219,12 @@

ai-helpers

} ], "skills": [ + { + "name": "git-commit-format", + "description": "Apply conventional commit formatting rules. Use when generating commit messages or creating commits.", + "description_html": "Apply conventional commit formatting rules. Use when generating commit messages or creating commits.", + "meta": "" + }, { "name": "suggest-reviewers", "description": "Git blame analysis helper for the suggest-reviewers command", @@ -2317,12 +2323,31 @@

ai-helpers

{ "name": "openshift-developer", "description": "Bundle of curated plugins, skills, and MCP servers useful to any OpenShift engineer", - "version": "1.0.0", + "version": "1.1.0", "has_readme": true, "commands": [], - "skills": [], + "skills": [ + { + "name": "address-review-pr", + "description": "Fetch and address all PR review comments — categorize by priority, make code changes, post replies, and push. Use when the user wants to address, respond to, or work through PR review feedback.", + "description_html": "Fetch and address all PR review comments — categorize by priority, make code changes, post replies, and push. Use when the user wants to address, respond to, or work through PR review feedback.", + "meta": "" + }, + { + "name": "address-review-precommit", + "description": "Fix code review findings before committing. Use when the user wants to address pre-commit review feedback, fix review findings in the current branch, or apply code review fixes and push.", + "description_html": "Fix code review findings before committing. Use when the user wants to address pre-commit review feedback, fix review findings in the current branch, or apply code review fixes and push.", + "meta": "" + } + ], "agents": [], - "hooks": [], + "hooks": [ + { + "event_type": "SessionStart", + "matcher": ".*", + "hooks_json": "[\n {\n \"command\": \"${CLAUDE_PLUGIN_ROOT}/scripts/ensure-precommit.sh\",\n \"type\": \"command\"\n }\n]" + } + ], "mcp_servers": [], "rules": [], "category": "bundle", @@ -2798,17 +2823,9 @@

ai-helpers

{ "name": "utils", "description": "A generic utilities plugin serving as a catch-all for various helper commands and agents", - "version": "0.0.11", + "version": "0.0.12", "has_readme": true, "commands": [ - { - "name": "address-reviews", - "full_name": "utils:address-reviews", - "description": "Fetch and address all PR review comments", - "description_html": "Fetch and address all PR review comments", - "synopsis": "/utils:address-reviews [PR number (optional - uses current branch if omitted)] [--preview]", - "body_html": "This command automates the process of addressing PR review comments by fetching all comments from a pull request, categorizing them by priority (blocking, change requests, questions, suggestions), and systematically addressing each one. It intelligently filters out outdated comments, bot-generated content, and oversized responses to optimize context usage. The command handles code changes, posts replies to reviewers, and maintains a clean git history by amending relevant commits rather than creating unnecessary new ones." - }, { "name": "auto-approve-konflux-prs", "full_name": "utils:auto-approve-konflux-prs", diff --git a/plugins/git/.claude-plugin/plugin.json b/plugins/git/.claude-plugin/plugin.json index 3219fa6d0..cd7ac7d0f 100644 --- a/plugins/git/.claude-plugin/plugin.json +++ b/plugins/git/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "git", "description": "Git workflow automation and utilities", - "version": "0.0.6", + "version": "0.0.7", "author": { "name": "github.com/openshift-eng" } diff --git a/plugins/git/skills/git-commit-format/SKILL.md b/plugins/git/skills/git-commit-format/SKILL.md new file mode 100644 index 000000000..f5a95b075 --- /dev/null +++ b/plugins/git/skills/git-commit-format/SKILL.md @@ -0,0 +1,137 @@ +--- +name: git-commit-format +description: "Apply conventional commit formatting rules. Use when generating commit messages or creating commits." +--- + +## Name +openshift-developer:git-commit-format + +## Synopsis +``` +/openshift-developer:git-commit-format +``` + +## Description +Applies conventional commit formatting rules when generating commit messages. This skill is loaded as context for other skills and workflows — it defines the commit message format, required footers, and validation rules. + +## Implementation + +### Commit Message Format + +``` +(): + +[optional body] + +[footers] +``` + +### Commit Types + +- **feat**: New features +- **fix**: Bug fixes +- **docs**: Documentation changes +- **style**: Code style changes (formatting, etc.) +- **refactor**: Code refactoring (no functional changes) +- **test**: Adding/updating tests +- **chore**: Maintenance tasks +- **build**: Build system or dependency changes +- **ci**: CI/CD changes +- **perf**: Performance improvements +- **revert**: Revert previous commit + +### Breaking Changes + +With `!` to draw attention: +``` +feat!: send email when product shipped +``` + +With `BREAKING CHANGE` footer: +``` +feat: allow config to extend other configs + +BREAKING CHANGE: `extends` key now used for extending config files +``` + +### Required Footers + +#### Signed-off-by + +**ALWAYS include `Signed-off-by`** footer with name and email. + +Get credentials in this priority order: +1. Environment variables: `$GIT_AUTHOR_NAME` and `$GIT_AUTHOR_EMAIL` +2. Git config: `git config user.name` and `git config user.email` +3. If neither configured, ask user to provide details + +#### Commit-Message-Assisted-by + +**ALWAYS include** when Claude assists with creating or generating the commit message: +``` +Commit-Message-Assisted-by: Claude (via Claude Code) +``` + +### Gitlint Validation Rules + +- Run `make run-gitlint` to validate commit messages (if the repo has a gitlint target) +- **Title line**: 120 characters maximum +- **Body line**: 140 characters maximum per line +- Use conventional commit format +- Include required footers (Signed-off-by) +- No trailing whitespace + +## Return Value +- **Commit message**: A conventional commit message with type, scope, description, body, and all required footers + +## Examples + +1. **Simple commit**: + ``` + docs: correct spelling of CHANGELOG + + Signed-off-by: Jane Doe + Commit-Message-Assisted-by: Claude (via Claude Code) + ``` + +2. **With scope**: + ``` + feat(azure): add workload identity support + + Signed-off-by: Jane Doe + Commit-Message-Assisted-by: Claude (via Claude Code) + ``` + +3. **Multi-paragraph with footers**: + ``` + fix: prevent racing of requests + + Introduce request ID and reference to latest request. Dismiss + incoming responses other than from latest request. + + Remove timeouts which were used to mitigate racing but are + obsolete now. + + Reviewed-by: John Smith + Refs: #123 + Signed-off-by: Jane Doe + Commit-Message-Assisted-by: Claude (via Claude Code) + ``` + +## Arguments +- None. This skill provides formatting rules as context for other skills and workflows. + +## Quick Checklist + +When creating commits: +- [ ] Use conventional commit format: `(): ` +- [ ] Title under 120 characters +- [ ] Body lines under 140 characters +- [ ] Include `Signed-off-by` footer +- [ ] Include `Commit-Message-Assisted-by: Claude (via Claude Code)` footer +- [ ] Validate with `make run-gitlint` if available +- [ ] Use "!" or `BREAKING CHANGE` for breaking changes + +## Reference + +Conventional Commits Specification: https://www.conventionalcommits.org/en/v1.0.0/#specification diff --git a/plugins/openshift-developer/.claude-plugin/plugin.json b/plugins/openshift-developer/.claude-plugin/plugin.json index cbd060df5..710788930 100644 --- a/plugins/openshift-developer/.claude-plugin/plugin.json +++ b/plugins/openshift-developer/.claude-plugin/plugin.json @@ -1,14 +1,15 @@ { "name": "openshift-developer", "description": "Bundle of curated plugins, skills, and MCP servers useful to any OpenShift engineer", - "version": "1.0.0", + "version": "1.1.0", "author": { "name": "github.com/openshift-eng" }, "dependencies": [ - { "name": "jira", "version": "^0.5.0" }, + { "name": "jira", "version": "^0.7.0" }, { "name": "ci", "version": "^0.0.42" }, { "name": "golang", "version": "^0.3.0" }, - { "name": "prodsec-skills", "marketplace": "prodsec-skills" } + { "name": "prodsec-skills", "marketplace": "prodsec-skills" }, + { "name": "git", "version": "^0.0.6" } ] } diff --git a/plugins/openshift-developer/README.md b/plugins/openshift-developer/README.md index 0a7c881af..cb91a7350 100644 --- a/plugins/openshift-developer/README.md +++ b/plugins/openshift-developer/README.md @@ -1,6 +1,29 @@ # openshift-developer -Bundle of curated plugins, skills, and MCP servers useful to any OpenShift engineer. +Executable workflows for OpenShift development. + +These workflows are meant to be a common engine for different consumption models: + +- Developer's laptop +- Shared infrastructure like Prow for the OCPBUG autofix platform +- Slack chai-bot + + Each skill and command is a self-contained unit of work that any of these environments can invoke identically. + +## Common workflows + +### Pre-PR (author loop) + +1. `/jira:solve` — Pick up a Jira issue, analyze it, implement the fix. +2. `/code-review:pre-commit-review` — Run a code review on the local changes before pushing. +3. `/openshift-developer:address-review-precommit` — Apply the review findings, run verification, commit, and push. + +### Post-PR (review loop) + +1. `/code-review:pr` — Review an open PR for correctness and improvements. +2. `/openshift-developer:address-review-pr` — Fetch reviewer comments, categorize by priority, make code changes, post replies, and push. + +Repeat steps 4-5 until the PR is approved. ## What's included @@ -10,11 +33,32 @@ Bundle of curated plugins, skills, and MCP servers useful to any OpenShift engin - `ci` — OpenShift CI / Prow job analysis - `golang` — Go development tools - `prodsec-skills` — Product security skills +- `git` — Git workflow automation and utilities + +### Skills + +- **jira:solve** — Pick up a Jira issue, analyze it, implement the fix, and open a PR. (via `jira` plugin) +- **git:git-commit-format** — Conventional commit formatting rules: types, scopes, required footers (Signed-off-by, Commit-Message-Assisted-by), and gitlint validation. (via `git` plugin) +- **code-review:pre-commit-review** — Run a code review on local changes before pushing. (via `code-review` plugin) +- **address-review-precommit** — Fix code review findings in the current branch before committing: applies fixes, runs verification, and pushes. +- **code-review:pr** — Review an open PR for correctness and improvements. (via `code-review` plugin) +- **address-review-pr** — Fetch and address all PR review comments: categorizes by priority, makes code changes, posts replies, and pushes. + +### Hooks + +- **ensure-precommit** — On `SessionStart`, installs pre-commit and pre-push hooks via `pre-commit` if the repo has a `.pre-commit-config.yaml`. Fails if `pre-commit` is not installed. Every commit and push is then gated by the repo's hooks at zero ongoing token cost. ### MCP Servers - **atlassian** — Atlassian MCP server (`https://mcp.atlassian.com/v1/mcp`) +## Prerequisites + +- `pre-commit` — hook manager (`pip install pre-commit` or `brew install pre-commit`) +- `gitlint` — commit message linter (`pip install gitlint`) +- `gopls` — Go language server (`go install golang.org/x/tools/gopls@latest`) +- `gh` — GitHub CLI, authenticated (`brew install gh`) + ## Installation Add the marketplaces (one-time): @@ -37,3 +81,4 @@ This bundle can also be installed via APM with `--target`: ```sh apm install openshift-eng/ai-helpers/plugins/openshift-developer --global --target cursor ``` + diff --git a/plugins/openshift-developer/hooks/hooks.json b/plugins/openshift-developer/hooks/hooks.json new file mode 100644 index 000000000..6ce2914ba --- /dev/null +++ b/plugins/openshift-developer/hooks/hooks.json @@ -0,0 +1,14 @@ +{ + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PLUGIN_ROOT}/scripts/ensure-precommit.sh" + } + ] + } + ] + } +} diff --git a/plugins/openshift-developer/scripts/ensure-precommit.sh b/plugins/openshift-developer/scripts/ensure-precommit.sh new file mode 100755 index 000000000..0d36786ea --- /dev/null +++ b/plugins/openshift-developer/scripts/ensure-precommit.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ ! -f .pre-commit-config.yaml ]; then + exit 0 +fi + +if ! command -v pre-commit &>/dev/null; then + echo "ERROR: pre-commit is required but not found on PATH" >&2 + exit 2 +fi + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +python3 "$SCRIPT_DIR/validate_precommit_config.py" || exit $? + +pre-commit install --hook-type pre-commit >&2 +pre-commit install --hook-type pre-push >&2 +echo "pre-commit hooks installed" >&2 diff --git a/plugins/openshift-developer/scripts/validate_precommit_config.py b/plugins/openshift-developer/scripts/validate_precommit_config.py new file mode 100755 index 000000000..73610ee24 --- /dev/null +++ b/plugins/openshift-developer/scripts/validate_precommit_config.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python3 +"""Validate .pre-commit-config.yaml against a trusted repo whitelist. + +Exit codes: + 0 — all repos are trusted + 1 — untrusted repos found (printed to stderr) + 2 — parse error +""" + +import sys +import yaml + +TRUSTED_REPOS = { + "https://github.com/pre-commit/pre-commit-hooks": [ + "check-merge-conflict", + "check-yaml", + "trailing-whitespace", + ], + "https://github.com/leaktk/gitleaks": None, # all hooks allowed +} + + +def main(): + try: + with open(".pre-commit-config.yaml", encoding="utf-8") as f: + cfg = yaml.safe_load(f) + except (OSError, yaml.YAMLError) as e: + print(f"ERROR: failed to parse .pre-commit-config.yaml: {e}", file=sys.stderr) + return 2 + + if not isinstance(cfg, dict): + print("ERROR: .pre-commit-config.yaml must contain a top-level mapping", file=sys.stderr) + return 2 + + blocked_repos = [] + blocked_hooks = [] + for r in cfg.get("repos", []): + repo = r.get("repo", "") + if repo == "local": + continue + if repo not in TRUSTED_REPOS: + blocked_repos.append(repo) + continue + allowed_hooks = TRUSTED_REPOS[repo] + if allowed_hooks is None: + continue + for hook in r.get("hooks", []): + hook_id = hook.get("id", "") + if hook_id not in allowed_hooks: + blocked_hooks.append(f"{repo} -> {hook_id}") + + if blocked_repos or blocked_hooks: + print("ERROR: .pre-commit-config.yaml contains untrusted entries:", file=sys.stderr) + for b in blocked_repos: + print(f" untrusted repo: {b}", file=sys.stderr) + for b in blocked_hooks: + print(f" untrusted hook: {b}", file=sys.stderr) + print("Local hooks (repo: local) are always allowed.", file=sys.stderr) + return 1 + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/plugins/openshift-developer/skills/address-review-pr/SKILL.md b/plugins/openshift-developer/skills/address-review-pr/SKILL.md new file mode 100644 index 000000000..b32126441 --- /dev/null +++ b/plugins/openshift-developer/skills/address-review-pr/SKILL.md @@ -0,0 +1,240 @@ +--- +name: address-review-pr +description: Fetch and address all PR review comments — categorize by priority, make code changes, post replies, and push. Use when the user wants to address, respond to, or work through PR review feedback. +--- + +## Name +openshift-developer:address-review-pr + +## Synopsis +``` +/openshift-developer:address-review-pr [PR number] [--preview] +``` + +## Description +Automates addressing PR review comments by fetching all comments from a pull request, categorizing them by priority (blocking, change requests, questions, suggestions), and systematically addressing each one. Intelligently filters out outdated comments, bot-generated content, and oversized responses to optimize context usage. Handles code changes, posts replies to reviewers, and maintains a clean git history by amending relevant commits rather than creating unnecessary new ones. + +## Implementation + +### Step 0: Checkout the PR Branch + +1. **Determine PR number**: Use `$1` if provided, otherwise `gh pr list --head ` +2. **Checkout**: Use `gh pr checkout ` if not already on the branch, then `git pull` +3. **Verify clean working tree**: Run `git status`. If uncommitted changes exist, ask user how to proceed + +### Step 1: Fetch PR Context + +1. **Fetch PR metadata with selective filtering**: + + a. **First pass - Get metadata only** (IDs, authors, lengths, URLs): + ```bash + # Get issue comments (general PR comments - main conversation) + gh pr view --json comments --jq '.comments | map({ + id, + author: .author.login, + length: (.body | length), + url, + createdAt, + type: "issue_comment" + })' + + # Get reviews (need REST API for numeric IDs) + gh api repos/{owner}/{repo}/pulls//reviews --jq 'map({ + id, + author: .user.login, + length: (.body | length), + state, + submitted_at, + type: "review" + })' + + # Get review comments (inline code comments) + gh api repos/{owner}/{repo}/pulls//comments --jq 'map({ + id, + author: .user.login, + length: (.body | length), + path, + line, + original_line, + created_at, + type: "review_comment" + })' + ``` + + b. **Apply filtering logic** (DO NOT fetch full body yet): + - Filter out: `line == null AND original_line == null` (truly orphaned review comments). **Keep** comments where `line == null` but `original_line != null` — these are valid comments on a stale diff hunk that still need attention. + - Filter out: `length > 5000` + - Filter out: CI/automation bots `author in ["openshift-ci-robot", "openshift-ci"]` (keep coderabbitai for code review insights) + - Keep track of filtered items and stats for reporting + + c. **Second pass - Fetch ONLY essential fields for kept items**: + ```bash + # For issue comments: + gh api repos/{owner}/{repo}/issues/comments/ --jq '{id, body, user: .user.login, created_at, url}' + + # For reviews: + gh api repos/{owner}/{repo}/pulls//reviews/ --jq '{id, body, user: .user.login, state, submitted_at}' + + # For review comments: + gh api repos/{owner}/{repo}/pulls/comments/ --jq '{id, body, user: .user.login, path, line, original_line, position, diff_hunk, created_at}' + ``` + + d. **Log filtering results**: + ``` + Fetched N/M comments (filtered out K large/bot comments saving ~X chars) + ``` + +2. **Fetch commit messages**: `gh pr view --json commits -q '.commits[] | "\(.messageHeadline)\n\n\(.messageBody)"'` + +3. Store ONLY the kept (filtered) comments for analysis + +### Step 2: Categorize and Prioritize Comments + +1. **Additional filtering** (for remaining fetched comments): + - Already resolved comments + - Pure acknowledgments ("LGTM", "Thanks!", etc.) + +2. **Categorize**: + - **ACTION_INSTRUCTION**: Repo-level operations — rebase, verify, squash, update branch, run tests. + - **BLOCKING**: Critical changes (security, bugs, breaking issues) + - **CHANGE_REQUEST**: Code improvements or refactoring + - **QUESTION**: Requests for clarification + - **SUGGESTION**: Optional improvements (nits, non-critical) + +3. **Group by context**: Group by file, then by proximity (within 10 lines) + +4. **Prioritize**: ACTION_INSTRUCTION > BLOCKING > CHANGE_REQUEST > QUESTION > SUGGESTION + +5. **Present summary**: Show counts by category and file groupings, ask user to confirm + +### Step 3: Address Comments + +#### Interactive Preview (`--preview`) + +When `--preview` is passed, preview each comment before acting: + +1. Show the reviewer's comment +2. Show your proposed action: code change diff, explanation, or decline reasoning +3. Show the draft reply you plan to post +4. **Wait for user approval** before proceeding + +#### Action Instructions + +Process ACTION_INSTRUCTION items first, before any code changes: + +1. **Rebase**: Determine the base remote and branch first: + ```bash + BASE_BRANCH=$(gh pr view --json baseRefName -q '.baseRefName') + BASE_REMOTE=$(git remote | grep -m1 '^upstream$') + if [ -z "$BASE_REMOTE" ]; then + BASE_REMOTE=$(git remote | grep -m1 '^origin$') + fi + BASE_REMOTE=${BASE_REMOTE:-origin} + git fetch "$BASE_REMOTE" && git rebase "$BASE_REMOTE/$BASE_BRANCH" + ``` +2. **Verify/Test**: Run the repo's verification commands. If the reviewer asks to "make sure X passes", run X and fix failures before continuing. +3. **Squash/restructure commits**: Follow the reviewer's instructions on commit organization. + +#### Grouped Comments + +When multiple comments relate to the same concern/fix: +- Make the code change once +- Track replies for EACH comment individually (posted in Step 4) + +#### Code Change Requests + +**a. Validate**: Analyze if the change is valid. Don't be afraid to reject it if it doesn't make sense. + +**b. If valid**: +- Implement changes and commit locally (do NOT push yet — batched in Step 4) +- Default to amending the relevant commit. New commit only for substantial new features beyond PR scope. +- Follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) format + +**c. If declining**: Prepare technical explanation (3-5 sentences) with file:line references + +**d. If unsure**: Ask user for clarification + +#### Clarification Requests + +- Prepare clear, detailed answer (2-4 sentences) with file:line references + +### Step 3.5: Pre-Push Verification + +1. **Detect verification commands** (first match): + - `Makefile` with `verify` target -> `make verify` + - `Makefile` with `lint` target -> `make lint` + - `go.mod` exists -> `go build ./...` and `go vet ./...` + - `package.json` with `lint` script -> `npm run lint` + +2. **Run verification** (15-minute timeout). Maximum 3 retry attempts. Do NOT push code that fails verification. + +### Step 4: Post Replies and Push + +#### 4a. Post all replies + +- **Template**: `Done. [1-line what changed]. [Optional 1-line why]` +- Post reply: + ``` + gh api repos/{owner}/{repo}/pulls//comments//replies -f body="" + ``` +- **All replies must include**: `---\n*AI-assisted response via Claude Code*` + +#### 4b. Push once + +```bash +git push +``` + +#### 4c. Verify push + +- Confirm `git log -1 --format='%H'` matches `git ls-remote origin ` +- If push cannot be verified, report the failure — replies have already been posted + +### Step 5: Summary + +Show: total comments found, filtered out, addressed with code changes, replied to, requiring user input. + +## Return Value +- **Summary table** of comments processed by category +- **Git push result** confirming all changes are on the remote + +## Examples + +1. **Address reviews on current branch's PR**: + ``` + /openshift-developer:address-review-pr + ``` + +2. **Address reviews on a specific PR**: + ``` + /openshift-developer:address-review-pr 1234 + ``` + +3. **Preview mode**: + ``` + /openshift-developer:address-review-pr 1234 --preview + ``` + +## Arguments +- `$1`: PR number (optional — uses current branch if omitted) +- `--preview`: Preview each comment's proposed action and reply before proceeding + +## Duplicate Prevention + +Before posting ANY reply, verify you haven't already responded: + +```bash +SKILL_DIR="$(dirname "$(find ~/.claude/plugins -type f -path "*/openshift-developer/skills/address-review-pr/check_replied.py" 2>/dev/null | sort | head -1)")" +python3 "$SKILL_DIR/check_replied.py" --type +``` + +Where `` is one of: `issue_comment`, `review_thread`, or `review_comment` + +**Exit code 1**: Skip — already replied. +**Exit code 2**: Check failed — do NOT post a reply. + +## Response Rules + +1. **One response per feedback**: Inline review comments reply inline only. General PR comments reply as general comment only. NEVER both. +2. **Code changes require explicit request**: Only modify code for imperative language ("change", "fix", "remove"). For questions — reply with explanation only. +3. **Check before acting**: Questions ("Why did you...?") get explanations, not code changes. diff --git a/plugins/utils/scripts/check_replied.py b/plugins/openshift-developer/skills/address-review-pr/check_replied.py similarity index 100% rename from plugins/utils/scripts/check_replied.py rename to plugins/openshift-developer/skills/address-review-pr/check_replied.py diff --git a/plugins/openshift-developer/skills/address-review-precommit/SKILL.md b/plugins/openshift-developer/skills/address-review-precommit/SKILL.md new file mode 100644 index 000000000..33995ac42 --- /dev/null +++ b/plugins/openshift-developer/skills/address-review-precommit/SKILL.md @@ -0,0 +1,73 @@ +--- +name: address-review-precommit +description: Fix code review findings before committing. Use when the user wants to address pre-commit review feedback, fix review findings in the current branch, or apply code review fixes and push. +--- + +## Name +openshift-developer:address-review-precommit + +## Synopsis +``` +/openshift-developer:address-review-precommit +``` + +## Description +Applies code review findings to the current branch by editing the code, running verification, and pushing the fixes. Designed to run after `/code-review:pre-commit-review` to close the pre-PR author loop. + +## Implementation + +### Step 1: Understand the review findings + +Parse the provided review findings and identify all actions and improvements that need to be addressed. + +### Step 2: Apply fixes + +Address all actions and improvements by editing the code. For each finding: + +1. Locate the relevant file and code +2. Apply the fix +3. Verify the fix is correct + +### Step 3: Verify + +Run verification to ensure nothing is broken: + +```bash +make test 2>&1 +make verify 2>&1 +``` + +- If `make verify` generates new files, commit those too and run `make verify` again to confirm it passes +- Maximum 3 retry attempts if verification fails — fix the issues and re-run +- If verification still fails after 3 attempts, stop and report to the user + +### Step 4: Commit and push + +1. Amend existing commits or create new commits as appropriate +2. Push the branch to origin: + ```bash + git push + ``` + +## Return Value +- **Verification result**: pass or fail with error details +- **Git push result**: confirming fixes are on the remote + +## Examples + +1. **Fix findings from a prior review step**: + ``` + /openshift-developer:address-review-precommit + ``` + The review findings are passed from the preceding `/code-review:pre-commit-review` output. + +## Arguments +- **REVIEW_FINDINGS**: The review findings to address (passed inline or from a prior review step) +- **SUBAGENT_PROMPT**: Optional additional instructions for the fixing agent + +## Guidelines + +- Fix every issue identified in the review — all actions and improvements +- Do NOT run commands that reveal git credentials like `git remote -v` or `git remote get-url origin` +- If verification generates new files, commit those and re-verify +- Commit all fixes and push to origin diff --git a/plugins/utils/.claude-plugin/plugin.json b/plugins/utils/.claude-plugin/plugin.json index e3889d658..eea6ee6d3 100644 --- a/plugins/utils/.claude-plugin/plugin.json +++ b/plugins/utils/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "utils", "description": "A generic utilities plugin serving as a catch-all for various helper commands and agents", - "version": "0.0.11", + "version": "0.0.12", "author": { "name": "github.com/openshift-eng" } diff --git a/plugins/utils/README.md b/plugins/utils/README.md index 503b93127..ba1987412 100644 --- a/plugins/utils/README.md +++ b/plugins/utils/README.md @@ -8,10 +8,6 @@ General-purpose utilities and helper commands for development workflows. Generate comprehensive test steps for one or more related GitHub PRs. -### `/utils:address-reviews` - -Process and address code review comments on pull requests. - ### `/utils:process-renovate-pr` Automate processing of Renovate dependency update PRs. diff --git a/plugins/utils/commands/address-reviews.md b/plugins/utils/commands/address-reviews.md deleted file mode 100644 index 07011912e..000000000 --- a/plugins/utils/commands/address-reviews.md +++ /dev/null @@ -1,322 +0,0 @@ ---- -description: Fetch and address all PR review comments -argument-hint: "[PR number (optional - uses current branch if omitted)] [--preview]" ---- - -## Name -utils:address-reviews - -## Synopsis -/utils:address-reviews [PR number (optional - uses current branch if omitted)] [--preview] - -## Description -This command automates the process of addressing PR review comments by fetching all comments from a pull request, categorizing them by priority (blocking, change requests, questions, suggestions), and systematically addressing each one. It intelligently filters out outdated comments, bot-generated content, and oversized responses to optimize context usage. The command handles code changes, posts replies to reviewers, and maintains a clean git history by amending relevant commits rather than creating unnecessary new ones. - -## Implementation - -### Step 0: Checkout the PR Branch - -1. **Determine PR number**: Use $ARGUMENTS if provided, otherwise `gh pr list --head ` -2. **Checkout**: Use `gh pr checkout ` if not already on the branch, then `git pull` -3. **Verify clean working tree**: Run `git status`. If uncommitted changes exist, ask user how to proceed - -### Step 1: Fetch PR Context - -1. **Fetch PR metadata with selective filtering**: - - a. **First pass - Get metadata only** (IDs, authors, lengths, URLs): - ```bash - # Get issue comments (general PR comments - main conversation) - gh pr view --json comments --jq '.comments | map({ - id, - author: .author.login, - length: (.body | length), - url, - createdAt, - type: "issue_comment" - })' - - # Get reviews (need REST API for numeric IDs) - gh api repos/{owner}/{repo}/pulls//reviews --jq 'map({ - id, - author: .user.login, - length: (.body | length), - state, - submitted_at, - type: "review" - })' - - # Get review comments (inline code comments) - gh api repos/{owner}/{repo}/pulls//comments --jq 'map({ - id, - author: .user.login, - length: (.body | length), - path, - line, - original_line, - created_at, - type: "review_comment" - })' - ``` - - b. **Apply filtering logic** (DO NOT fetch full body yet): - - Filter out: `line == null AND original_line == null` (truly orphaned review comments). **Keep** comments where `line == null` but `original_line != null` — these are valid comments on a stale diff hunk that still need attention. - - Filter out: `length > 5000` - - Filter out: CI/automation bots `author in ["openshift-ci-robot", "openshift-ci"]` (keep coderabbitai for code review insights) - - Keep track of filtered items and stats for reporting - - c. **Second pass - Fetch ONLY essential fields for kept items**: - ```bash - # For issue comments - fetch only body and minimal metadata: - gh api repos/{owner}/{repo}/issues/comments/ --jq '{id, body, user: .user.login, created_at, url}' - - # For reviews - fetch only body and state: - gh api repos/{owner}/{repo}/pulls//reviews/ --jq '{id, body, user: .user.login, state, submitted_at}' - - # For review comments - fetch only body and code context: - gh api repos/{owner}/{repo}/pulls/comments/ --jq '{id, body, user: .user.login, path, line, original_line, position, diff_hunk, created_at}' - ``` - - **Note**: Using `--jq` to select only needed fields minimizes context usage. Avoid fetching full API responses with all metadata. - - d. **Log filtering results**: - ``` - ℹī¸ Fetched N/M comments (filtered out K large/bot comments saving ~X chars) - ``` - -2. **Fetch commit messages**: `gh pr view --json commits -q '.commits[] | "\(.messageHeadline)\n\n\(.messageBody)"'` - -3. Store ONLY the kept (filtered) comments for analysis - -### Step 2: Categorize and Prioritize Comments - -**Note**: Most filtering already happened in Step 1 to save context window space. - -1. **Additional filtering** (for remaining fetched comments): - - Already resolved comments - - Pure acknowledgments ("LGTM", "Thanks!", etc.) - -2. **Categorize**: - - **ACTION_INSTRUCTION**: Repo-level operations — rebase, verify, squash, update branch, run tests. These are NOT code review comments; they are instructions to perform an operation on the branch itself. Common patterns: "please rebase", "make sure verify passes", "squash commits", "run tests before pushing". - - **BLOCKING**: Critical changes (security, bugs, breaking issues) - - **CHANGE_REQUEST**: Code improvements or refactoring - - **QUESTION**: Requests for clarification - - **SUGGESTION**: Optional improvements (nits, non-critical) - -3. **Group by context**: Group by file, then by proximity (within 10 lines) - -4. **Prioritize**: ACTION_INSTRUCTION → BLOCKING → CHANGE_REQUEST → QUESTION → SUGGESTION - - ACTION_INSTRUCTION items run first because they affect the branch state that all subsequent work builds on (e.g. rebase before making code changes) - -5. **Present summary**: Show counts by category and file groupings, ask user to confirm - -### Step 3: Address Comments - -#### Interactive Preview (`--preview`) - -When `--preview` is passed, preview each comment before acting: - -1. Show the reviewer's comment -2. Show your proposed action: code change diff, explanation, or decline reasoning -3. Show the draft reply you plan to post -4. **Wait for user approval** before proceeding — the user can: - - **Approve** as-is - - **Edit** the proposed reply or approach - - **Skip** the comment entirely - -This applies to all comment types below. Without `--preview`, act autonomously. - -#### Action Instructions - -Process ACTION_INSTRUCTION items first, before any code changes: - -1. **Rebase**: Determine the base remote and branch first: - ```bash - BASE_BRANCH=$(gh pr view --json baseRefName -q '.baseRefName') - # Prefer 'upstream' remote, then 'origin', then the current branch's tracking remote - BASE_REMOTE=$(git remote | grep -m1 '^upstream$') - if [ -z "$BASE_REMOTE" ]; then - BASE_REMOTE=$(git remote | grep -m1 '^origin$') - fi - if [ -z "$BASE_REMOTE" ]; then - BASE_REMOTE=$(git rev-parse --abbrev-ref --symbolic-full-name '@{u}' 2>/dev/null | cut -d/ -f1) - fi - BASE_REMOTE=${BASE_REMOTE:-origin} - git fetch "$BASE_REMOTE" && git rebase "$BASE_REMOTE/$BASE_BRANCH" - ``` - Resolve conflicts if any. After rebase, line numbers and diff hunks from pre-rebase comments may be stale — resolve them against the current file state rather than trusting literally. -2. **Verify/Test**: Run the repo's verification commands (see Step 3.5 for detection). If the reviewer asks to "make sure X passes", run X and fix failures before continuing. -3. **Squash/restructure commits**: Follow the reviewer's instructions on commit organization. -4. **Other**: Execute the requested operation. If unclear, ask the user. - -After completing action instructions, reply to each one confirming what was done. - -#### Grouped Comments - -When multiple comments relate to the same concern/fix: -- Make the code change once -- Track replies for EACH comment individually (posted in Step 4 — don't copy-paste, tailor each reply) -- Optional reference: `Done. (Also addresses feedback from @user)` - -#### Code Change Requests - -**a. Validate**: Thoroughly analyze if the change is valid and fixes an issue or improves code. Don't be afraid to reject the change if it doesn't make sense. - -**b. If requested change is valid**: -- Plan and implement changes -- Commit locally **(do NOT push yet — all pushes are batched in Step 4)** - 1. **Review changes**: `git diff` - - 2. **Sync with remote first**: `git pull --rebase origin ` to ensure local branch is up to date. If the branch is behind or diverged, you MUST rebase before committing. - - 3. **Analyze commit structure**: `git log --oneline origin/main..HEAD` - - Identify which commit the changes relate to - - 4. **Commit strategy**: - - **DEFAULT: Amend the relevant commit** - - - ✅ **AMEND**: Review fixes, bug fixes, style improvements, refactoring, docs, tests within PR scope - - ❌ **NEW COMMIT**: Only for substantial new features beyond PR's original scope - - **When unsure**: Amend (keep git history clean) - - **Multiple commits**: Use `git rebase -i origin/main` to amend the specific relevant commit - - 5. **Create commit locally**: - - Follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) format - - Always include body explaining "why" - - **Amend**: `git commit --amend --no-edit` (or update message if scope changed) - - **New commit**: Standard commit with message - -- Track what was done for each comment (change description, comment ID, author) so replies can be posted in Step 4 - -**c. If declining change**: -- **Prepare technical explanation** (3-5 sentences): - - Why current implementation is correct - - Specific reasoning with file:line references -- Track for reply in Step 4 - -**d. If unsure**: Ask user for clarification - -#### Clarification Requests - -- Prepare clear, detailed answer (2-4 sentences) -- Include file:line references when applicable -- Track for reply in Step 4 - -#### Informational Comments - -- No action unless response is courteous - -### Step 3.5: Pre-Push Verification - -Before posting replies or pushing, verify the changes compile and pass basic checks. - -1. **Detect available verification commands** (check in order, use the first that exists): - - `Makefile` or `makefile` with a `verify` target → `make verify` - - `Makefile` with a `lint` target → `make lint` - - `go.mod` exists → `go build ./...` and `go vet ./...` - - `package.json` with a `lint` script → `npm run lint` - - If none found, skip verification but log: `⚠ī¸ No verification command detected — skipping pre-push verification` - -2. **Run verification** (15-minute timeout): - ```bash - # Example for a Go repo with Makefile (15-minute timeout): - timeout 15m make verify 2>&1 - ``` - - If verification fails: fix the issues, amend the relevant commit, and re-run verification - - Maximum 3 retry attempts. If verification still fails after 3 fix-and-retry cycles, stop and report to the user: "Verification continues to fail after 3 attempts. Last error: [error]. Manual intervention needed." - - Do NOT push code that fails verification - -3. **Log result**: - ```text - ✅ Verification passed (make verify) - ``` - or - ```text - ❌ Verification failed — fixing issues before push - ``` - -### Step 4: Post Replies and Push - -After ALL comments from Step 3 are processed, post replies and push in this order: - -#### 4a. Post all replies - -For each comment addressed in Step 3, post the reply: - -- **Concise Reply template**: `Done. [1-line what changed]. [Optional 1-line why]` - - Max 2 sentences + attribution footer -- Post reply: - ``` - gh api repos/{owner}/{repo}/pulls//comments//replies -f body="" - ``` - If fails: `gh pr comment --body="@ "` - -**All replies must include**: `---\n*AI-assisted response via Claude Code*` - -#### 4b. Push once - -After all replies are posted, push all committed changes in a single push: - -```bash -git push --force-with-lease -``` - -#### 4c. Verify push - -- Run `git log -1 --format='%H'` locally and `git ls-remote origin ` to confirm the remote has your commit -- **If they differ**: The push failed or was never executed. Diagnose and retry. -- **If uncommitted changes remain** (`git status`): The commit failed. Fix it before pushing. -- **If push cannot be verified**: Report the failure to the user. Do not silently proceed — replies have already been posted claiming changes were made. - -### Step 5: Summary - -Show user: -- Total comments found (raw count from API) -- Comments filtered out (with reason: outdated/large/bot-generated) -- Comments addressed with code changes -- Comments replied to -- Comments requiring user input - -## Guidelines - -- Address every non-filtered comment before finishing — do not skip comments silently -- Maintain professional tone in all replies -- Prioritize code quality over quick fixes -- Ensure code builds and passes tests after changes -- When in doubt, ask the user -- Use TodoWrite to track progress through multiple comments - -## Duplicate Prevention - -Before posting ANY reply, verify you haven't already responded: - -```bash -CHECK_REPLIED="${CLAUDE_PLUGIN_ROOT}/scripts/check_replied.py" -if [ ! -f "$CHECK_REPLIED" ]; then - CHECK_REPLIED=$(find ~/.claude/plugins -type f -path "*/utils/scripts/check_replied.py" 2>/dev/null | sort | head -1) -fi -if [ -z "$CHECK_REPLIED" ] || [ ! -f "$CHECK_REPLIED" ]; then echo "ERROR: check_replied.py not found" >&2; exit 2; fi -python3 "$CHECK_REPLIED" --type -``` - -Where `` is one of: `issue_comment`, `review_thread`, or `review_comment` - -**If the script returns exit code 1**: Skip that comment - you've already replied. -**If the script returns exit code 2**: The check failed - do NOT post a reply. Investigate and fix the issue before proceeding. - -### Response Rules - -1. **One response per feedback**: For each piece of feedback, choose ONE response mechanism: - - Inline review comments → reply inline only - - General PR comments → reply as general comment only - - NEVER respond to the same feedback via both mechanisms - -2. **Code changes require explicit request**: Only modify code when the reviewer explicitly asks using imperative language like "change", "fix", "remove", "update", "add". For questions, clarifications, or observations - reply with explanation only, do not change code. - -3. **Check before acting**: If a comment is phrased as a question ("Why did you...?", "What about...?"), provide an explanation. Only make code changes for direct requests ("Please change...", "This should be...", "Remove this..."). - - -## Arguments: -- $1: [PR number (optional - uses current branch if omitted)] -- --preview: Preview each comment's proposed action and reply before proceeding \ No newline at end of file