Skip to content

openshift-developer: add hooks, skills, and workflow documentation - #554

Merged
openshift-merge-bot[bot] merged 8 commits into
openshift-eng:mainfrom
enxebre:port-git-hooks-to-openshift-dev
Jun 22, 2026
Merged

openshift-merge-bot[bot] merged 8 commits into
openshift-eng:mainfrom
enxebre:port-git-hooks-to-openshift-dev

Conversation

@enxebre

@enxebre enxebre commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add SessionStart hook that installs pre-commit and pre-push hooks via pre-commit when a repo has .pre-commit-config.yaml
  • Add address-review-pr skill (moved from utils:address-reviews) — fetches and addresses PR review comments by priority
  • Add address-review-precommit skill — fixes code review findings before committing
  • Add git-commit-format skill — conventional commit formatting rules (ported from openshift/hypershift)
  • Document pre-PR and post-PR workflows in README
  • Add prerequisites section (pre-commit, gitlint, gopls, gh)
  • Bump version to 1.1.0

Test plan

  • Install plugin: claude plugin install openshift-developer@ai-helpers
  • Verify /openshift-developer:address-review-pr is available
  • Verify /openshift-developer:address-review-precommit is available
  • Verify /openshift-developer:git-commit-format is available
  • Verify SessionStart hook runs in a repo with .pre-commit-config.yaml
  • make lint passes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added OpenShift Developer skills to help address PR review feedback and run review-precommit fixes.
    • Introduced a git-commit-format skill for Conventional Commit-style messages.
    • Added an automatic pre-commit hook setup step when a pre-commit config is present.
  • Chores
    • Bumped plugin versions (git, utils, OpenShift Developer) and updated OpenShift Developer’s Jira dependency range.
  • Removals
    • Removed the utils:address-reviews command documentation.

@openshift-ci
openshift-ci Bot requested review from celebdor and stbenjam June 17, 2026 16:50
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 17, 2026
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9f06a22f-3f0b-4d30-83af-605df40cdb98

📥 Commits

Reviewing files that changed from the base of the PR and between 1f39082 and c3a95fc.

📒 Files selected for processing (1)
  • plugins/openshift-developer/scripts/validate_precommit_config.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugins/openshift-developer/scripts/validate_precommit_config.py

Walkthrough

The PR adds two new skills (address-review-pr, address-review-precommit) to the openshift-developer plugin, introduces a SessionStart hook that validates and installs pre-commit hooks via a new shell script and Python validator, adds a git-commit-format skill to the git plugin, removes the superseded utils:address-reviews command, and bumps versions for all three affected plugins.

Changes

Feature release: new review skills, precommit hooks, and registry updates

Layer / File(s) Summary
SessionStart pre-commit hook and config validator
plugins/openshift-developer/hooks/hooks.json, plugins/openshift-developer/scripts/ensure-precommit.sh, plugins/openshift-developer/scripts/validate_precommit_config.py, .skillsaw.yaml
hooks.json registers a SessionStart command hook running ensure-precommit.sh; the script conditionally validates .pre-commit-config.yaml against a TRUSTED_REPOS whitelist via validate_precommit_config.py and installs pre-commit and pre-push hook types; .skillsaw.yaml allowlist is updated to permit the new script path.
address-review-pr skill specification
plugins/openshift-developer/skills/address-review-pr/SKILL.md
Full skill spec for openshift-developer:address-review-pr: two-pass GitHub API comment fetching with bot/size/orphan filtering, five-category prioritization, --preview interactive gating, rebase and code-change handling, pre-push verification with timeout/retry, GitHub API reply posting with required attribution footer, push-and-verify flow, and duplicate-prevention via check_replied.py.
address-review-precommit skill specification
plugins/openshift-developer/skills/address-review-precommit/SKILL.md
New openshift-developer:address-review-precommit skill: four-step flow parsing prior review findings, applying and verifying fixes, running make test/make verify with up to 3 retries (committing generated files from make verify), then amending/creating commits and pushing.
git-commit-format skill and git plugin bump
plugins/git/skills/git-commit-format/SKILL.md, plugins/git/.claude-plugin/plugin.json
Adds git-commit-format SKILL.md specifying Conventional Commits format, supported types, breaking-change syntax, required Signed-off-by and conditional Claude attribution footers, gitlint validation rules, and examples. Bumps git plugin manifest to 0.0.7.
utils:address-reviews removal and utils version bump
plugins/utils/commands/address-reviews.md (deleted), plugins/utils/README.md, plugins/utils/.claude-plugin/plugin.json
Removes the utils:address-reviews command file and its README entry; bumps utils plugin manifest to 0.0.12.
openshift-developer manifest and dependency updates
plugins/openshift-developer/.claude-plugin/plugin.json
Bumps openshift-developer manifest to 1.1.0, updates jira dependency to ^0.7.0.
openshift-developer README expansion
plugins/openshift-developer/README.md
Expands README with Common workflows section (Pre-PR and Post-PR command sequences), detailed Skills descriptions, Hooks section documenting ensure-precommit behavior, clarified Prerequisites tooling list, and updated MCP Servers section.
Marketplace and docs index version synchronization
.claude-plugin/marketplace.json, docs/index.html
Propagates all version bumps (git 0.0.7, utils 0.0.12, openshift-developer 1.1.0) and new skill/hook definitions (git-commit-format, address-review-pr, address-review-precommit, SessionStart hook) to marketplace registry and docs index.

Sequence Diagram(s)

sequenceDiagram
    rect rgba(173, 216, 230, 0.5)
        Note over Claude,GitHub API: address-review-pr workflow
    end
    participant Claude as Claude (address-review-pr)
    participant GitHub API as GitHub API
    participant repo as Local Git Repo
    participant check_replied as check_replied.py

    Claude->>GitHub API: Pass 1 — fetch comment/review metadata (IDs, authors, body lengths)
    GitHub API-->>Claude: Raw comment list
    Claude->>Claude: Filter bots, oversized bodies (>5000), orphaned items
    Claude->>GitHub API: Pass 2 — fetch essential fields for kept items + PR commit messages
    GitHub API-->>Claude: Filtered comment details
    Claude->>Claude: Categorize into ACTION_INSTRUCTION/BLOCKING/CHANGE_REQUEST/QUESTION/SUGGESTION
    Claude->>Claude: --preview: display proposed action per comment, await user confirmation
    Claude->>repo: Rebase, apply code changes, run verification (15 min timeout, 3 retries)
    Claude->>check_replied: Check for prior replies (exit code gating)
    check_replied-->>Claude: No duplicates confirmed
    Claude->>GitHub API: Post replies with attribution footer
    Claude->>repo: git push (once)
    Claude->>GitHub API: git ls-remote origin branch — verify pushed commit hash
    Claude->>Claude: Emit final summary (counts + push confirmation)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • openshift-eng/ai-helpers#354: The push-and-remote-verify flow in the new address-review-pr skill directly matches the fix introduced in this PR for utils:address-reviews.
  • openshift-eng/ai-helpers#415: Both PRs directly touch plugins/utils/commands/address-reviews.md#415 added --preview and push-flow changes to that file, which this PR now deletes.
  • openshift-eng/ai-helpers#425: Modifies plugins/utils/commands/address-reviews.md for reliability improvements in the same file this PR removes.

Suggested labels

lgtm

Suggested reviewers

  • bentito
  • bryan-cox

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 1 warning)

Check name Status Explanation Resolution
No Assumed Git Remote Names ❌ Error Skills hardcode git remote names without discovery: address-review-pr uses git ls-remote origin <branch> (line 190) and fallback BASE_REMOTE=${BASE_REMOTE:-origin} (line 132); address-review-pr... Replace hardcoded "origin" with discovered remote names. Use git remote or git branch -vv to discover the actual remote before using it in git commands like ls-remote, push, and fetch.
Git Push Safety Rules ❌ Error The address-review-pr and address-review-precommit skills execute git push without explicit user permission. address-review-pr only asks user to confirm which comments to address in Step 2, but n... Add confirmation prompt before each git push command: "Ready to push changes. Proceed? (yes/no)". Add branch validation to reject pushes to main/master. Ensure user explicitly approves the push operation.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding hooks, skills, and workflow documentation to the openshift-developer plugin, which aligns with the significant changes across multiple files including hooks/hooks.json, new SKILL.md files, README updates, and version bumps.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Real People Names In Style References ✅ Passed No real people names used as style references found. Only placeholder names (Jane Doe, John Smith) in example commit message footers, not style guidance.
No Untrusted Mcp Servers ✅ Passed No untrusted MCP server installations found. All dependencies reference established RedHat plugins and authorized marketplaces; no mcpServers config added, no npm/npx/uvx MCP package installations...
Ai-Helpers Overlap Detection ✅ Passed No overlapping ai-helpers functionality detected. PR refactors utils:address-reviews into complementary openshift-developer skills (address-review-pr, address-review-precommit) and adds git-commit-...
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/openshift-developer/README.md`:
- Line 26: The instruction on line 26 references "steps 4-5" but the Post-PR
workflow section (lines 21-24) only defines 2 steps. Update the text "Repeat
steps 4-5 until the PR is approved" to "Repeat steps 1-2 until the PR is
approved" to correctly reference the review cycle steps that were actually
defined in the Post-PR workflow section.

In `@plugins/openshift-developer/skills/address-review-pr/SKILL.md`:
- Line 185: The SKILL.md file at line 185 documents the use of git push
--force-with-lease, which violates the coding guideline that prohibits all force
push operations including --force-with-lease variants. Replace the documented
command git push --force-with-lease with a safe push strategy such as standard
git push without the force flag, and ensure any accompanying documentation
explains the safer approach for pushing changes to the remote branch.
- Around line 171-191: The Step 4 workflow section (Post Replies and Push) does
not include an explicit user confirmation step before executing the git push
command in section 4b. According to the coding guideline, commands must never
push without explicit user permission. Add a new confirmation step between 4a
(Post all replies) and 4b (Push once) that asks the user to explicitly approve
the push action before proceeding with the git push --force-with-lease command.
This confirmation should be a required step in the main workflow, independent of
any --preview mode flag.
- Around line 171-191: In the Step 4b "Push once" section, add a pre-push
validation step before the git push command that retrieves the current branch
name and explicitly checks if it matches main or master variants. If the current
branch is a protected branch like main or master, exit with an error message and
non-zero exit code to prevent the push. This validation should occur immediately
before the git push --force-with-lease command to ensure no push operation
occurs to protected branches.
- Around line 128-132: The BASE_REMOTE discovery logic uses hardcoded
assumptions by grepping for specific remote names (upstream and origin) instead
of discovering them dynamically. Replace the grep-based approach that searches
for hardcoded remote names with a safe discovery method that uses git branch -vv
to extract the actual remote tracking the current branch, parsing the remote
name from the tracking information. If no tracking remote is found, fall back to
using git remote | head -1 to select the first available remote rather than
assuming a default name, ensuring the code discovers remotes first before making
assumptions about their names.

In `@plugins/openshift-developer/skills/address-review-precommit/SKILL.md`:
- Line 49: The documentation contains the command git push --force-with-lease
which violates the force-push ban guideline. Locate the git push
--force-with-lease command in the SKILL.md file and replace it with a standard
git push command to comply with the coding guideline that rejects all force-push
variants.
- Around line 44-50: Step 4 in the SKILL.md file needs to add two safety checks
before executing the git push command: first, add a branch verification step
that checks if the current branch is main or master using git rev-parse and
prevents the push if it is, then add an explicit user confirmation prompt asking
the user to approve the push before executing the git push --force-with-lease
command. Update the step numbering accordingly and ensure both the branch
protection check and user confirmation are clearly documented in the markdown
instructions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b439e3ae-7170-4a75-9b46-e1ee3165d7bb

📥 Commits

Reviewing files that changed from the base of the PR and between 193a32f and d00df0b.

📒 Files selected for processing (14)
  • .claude-plugin/marketplace.json
  • .skillsaw.yaml
  • docs/index.html
  • plugins/openshift-developer/.claude-plugin/plugin.json
  • plugins/openshift-developer/README.md
  • plugins/openshift-developer/hooks/hooks.json
  • plugins/openshift-developer/scripts/ensure-precommit.sh
  • plugins/openshift-developer/skills/address-review-pr/SKILL.md
  • plugins/openshift-developer/skills/address-review-pr/check_replied.py
  • plugins/openshift-developer/skills/address-review-precommit/SKILL.md
  • plugins/openshift-developer/skills/git-commit-format/SKILL.md
  • plugins/utils/.claude-plugin/plugin.json
  • plugins/utils/README.md
  • plugins/utils/commands/address-reviews.md
💤 Files with no reviewable changes (2)
  • plugins/utils/commands/address-reviews.md
  • plugins/utils/README.md

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the step reference in the Post-PR workflow iteration instruction.

Line 26 references "steps 4-5", but the Post-PR workflow (lines 21–24) only defines 2 steps. This appears to be a stale or copy-pasted reference.

Based on the workflow, the repetition instruction should likely be "Repeat steps 1-2 until the PR is approved" (i.e., iterate the review cycle: code review → address comments → push).

🔧 Proposed fix
-Repeat steps 4-5 until the PR is approved.
+Repeat steps 1-2 until the PR is approved.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Repeat steps 4-5 until the PR is approved.
Repeat steps 1-2 until the PR is approved.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/openshift-developer/README.md` at line 26, The instruction on line 26
references "steps 4-5" but the Post-PR workflow section (lines 21-24) only
defines 2 steps. Update the text "Repeat steps 4-5 until the PR is approved" to
"Repeat steps 1-2 until the PR is approved" to correctly reference the review
cycle steps that were actually defined in the Post-PR workflow section.

Comment on lines +128 to +132
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}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use safe git commands to discover remotes instead of grepping hardcoded names.

Lines 128–132 attempt to discover the base remote by grepping for hardcoded names (upstream, then origin). The coding guideline requires: "Code should NEVER assume git remote names like 'origin' or 'upstream' without first discovering them — use git remote -v or git branch -vv to discover remotes first." While the current logic does fall back to a default, the initial grep assumes names without discovery.

🔍 Proposed fix: Use safe remote discovery
 1. **Rebase**: Determine the base remote and branch first:
    ```bash
    BASE_BRANCH=$(gh pr view <PR_NUMBER> --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}
+   # Discover the actual remote tracking the base branch
+   BASE_REMOTE=$(git branch -vv | grep "^\* " | grep -oP '\[\K[^/]+' | head -1)
+   if [ -z "$BASE_REMOTE" ]; then
+     BASE_REMOTE=$(git remote | head -1)
+   fi
    git fetch "$BASE_REMOTE" && git rebase "$BASE_REMOTE/$BASE_BRANCH"
    ```
🧰 Tools
🪛 SkillSpector (2.1.1)

[error] 185: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).

Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.

(Tool Misuse (TM1))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/openshift-developer/skills/address-review-pr/SKILL.md` around lines
128 - 132, The BASE_REMOTE discovery logic uses hardcoded assumptions by
grepping for specific remote names (upstream and origin) instead of discovering
them dynamically. Replace the grep-based approach that searches for hardcoded
remote names with a safe discovery method that uses git branch -vv to extract
the actual remote tracking the current branch, parsing the remote name from the
tracking information. If no tracking remote is found, fall back to using git
remote | head -1 to select the first available remote rather than assuming a
default name, ensuring the code discovers remotes first before making
assumptions about their names.

Source: Coding guidelines

Comment on lines +171 to +191
### 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/<PR_NUMBER>/comments/<comment_id>/replies -f body="<reply>"
```
- **All replies must include**: `---\n*AI-assisted response via Claude Code*`

#### 4b. Push once

```bash
git push --force-with-lease
```

#### 4c. Verify push

- Confirm `git log -1 --format='%H'` matches `git ls-remote origin <branch>`
- If push cannot be verified, report the failure — replies have already been posted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clarify user permission flow for pushing — step 4 requires explicit re-confirmation.

Step 4 (Post Replies and Push) shows replies being posted and then a push, but the workflow does not explicitly re-confirm with the user before the actual push happens. The coding guideline requires: "Commands and skills must NEVER push without explicit user permission — always ask the user before pushing." The --preview mode exists (lines 112–119) but only applies when that flag is passed; the main workflow should also ask for final confirmation before pushing.

✅ Proposed clarification: Add explicit re-confirmation step before push
 #### 4b. Push once

+Before pushing, ask the user: "Ready to push all changes? [y/N]"
+If user declines, skip the push and report what would have been pushed.
+
 ```bash
 git push
</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 177-177: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>
<details>
<summary>🪛 SkillSpector (2.1.1)</summary>

[error] 185: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).

Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.

(Tool Misuse (TM1))

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @plugins/openshift-developer/skills/address-review-pr/SKILL.md around lines
171 - 191, The Step 4 workflow section (Post Replies and Push) does not include
an explicit user confirmation step before executing the git push command in
section 4b. According to the coding guideline, commands must never push without
explicit user permission. Add a new confirmation step between 4a (Post all
replies) and 4b (Push once) that asks the user to explicitly approve the push
action before proceeding with the git push --force-with-lease command. This
confirmation should be a required step in the main workflow, independent of any
--preview mode flag.


</details>

<!-- fingerprinting:phantom:triton:mongoose -->

<!-- cr-comment:v1:87870e1382f0c2d4c9b07404 -->

_Source: Coding guidelines_

---

_⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_

**Add explicit protection against pushing to main or master branches.**

The skill works on a PR branch (checked out at line 22), but the Step 4 push does not explicitly verify that the target branch is not a protected branch like `main` or `master`. The coding guideline states: "NEVER allow pushing to main or master branches — reject any `git push` command targeting main/master variants." Add a pre-push validation step.





<details>
<summary>🛡️ Proposed fix: Verify branch before push</summary>

```diff
 #### 4b. Push once

+Verify the current branch is not a protected branch:
+```bash
+CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
+if [[ "$CURRENT_BRANCH" =~ ^(main|master)$ ]]; then
+  echo "ERROR: Cannot push to protected branch '$CURRENT_BRANCH'" >&2
+  exit 1
+fi
+```
+
 ```bash
 git push
</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 177-177: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>
<details>
<summary>🪛 SkillSpector (2.1.1)</summary>

[error] 185: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).

Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.

(Tool Misuse (TM1))

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @plugins/openshift-developer/skills/address-review-pr/SKILL.md around lines
171 - 191, In the Step 4b "Push once" section, add a pre-push validation step
before the git push command that retrieves the current branch name and
explicitly checks if it matches main or master variants. If the current branch
is a protected branch like main or master, exit with an error message and
non-zero exit code to prevent the push. This validation should occur immediately
before the git push --force-with-lease command to ensure no push operation
occurs to protected branches.


</details>

<!-- fingerprinting:phantom:triton:mongoose -->

<!-- cr-comment:v1:91f5cc289b2c123ac571324c -->

_Source: Coding guidelines_

<!-- This is an auto-generated comment by CodeRabbit -->

Comment thread plugins/openshift-developer/skills/address-review-pr/SKILL.md Outdated
Comment on lines +44 to +50
### Step 4: Commit and push

1. Amend existing commits or create new commits as appropriate
2. Push the branch to origin:
```bash
git push --force-with-lease
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add explicit re-confirmation before push and protect against pushing to main/master.

Step 4 lacks an explicit user permission prompt before pushing, and does not verify the current branch is not a protected branch (main/master). The coding guidelines require: (1) "always ask the user before pushing", and (2) "NEVER allow pushing to main or master branches."

✅ Proposed fix: Add confirmation and branch protection
 ### Step 4: Commit and push

 1. Amend existing commits or create new commits as appropriate
+2. Verify the current branch is not a protected branch:
+   ```bash
+   CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
+   if [[ "$CURRENT_BRANCH" =~ ^(main|master)$ ]]; then
+     echo "ERROR: Cannot push to protected branch '$CURRENT_BRANCH'" >&2
+     exit 1
+   fi
+   ```
-2. Push the branch to origin:
+3. Ask the user: "Ready to push fixes to origin? [y/N]"
+   If user declines, skip the push and report what was staged.
+4. Push the branch to origin:
    ```bash
    git push
    ```
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### Step 4: Commit and push
1. Amend existing commits or create new commits as appropriate
2. Push the branch to origin:
```bash
git push --force-with-lease
```
### Step 4: Commit and push
1. Amend existing commits or create new commits as appropriate
2. Verify the current branch is not a protected branch:
🧰 Tools
🪛 SkillSpector (2.1.1)

[error] 49: [TM1] Tool Parameter Abuse: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).

Remediation: Validate all tool parameters against an allowlist. Reject dangerous parameter values (shell=True, --force, -rf /) and use safe defaults.

(Tool Misuse (TM1))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/openshift-developer/skills/address-review-precommit/SKILL.md` around
lines 44 - 50, Step 4 in the SKILL.md file needs to add two safety checks before
executing the git push command: first, add a branch verification step that
checks if the current branch is main or master using git rev-parse and prevents
the push if it is, then add an explicit user confirmation prompt asking the user
to approve the push before executing the git push --force-with-lease command.
Update the step numbering accordingly and ensure both the branch protection
check and user confirmation are clearly documented in the markdown instructions.

Source: Coding guidelines

Comment thread plugins/openshift-developer/skills/address-review-precommit/SKILL.md Outdated
Comment thread plugins/openshift-developer/scripts/ensure-precommit.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How does this affect the periodic and the GHA we have now that runs this plugin?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The intent is to make this bundle the main way we distribute the executable dev workflows everywhere. They'll need to be updated

1. Amend existing commits or create new commits as appropriate
2. Push the branch to origin:
```bash
git push --force-with-lease

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 to coderabbit here, I try to keep my claude from ever force pushing, I can use tide's autosquash to squash things

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the original intent was for this skills to ammend commits where it make sense, hence the need to push force.
I can also see value on a workflow that always commit/push atop and let the commit/squash be a separate step. Dropped the --force flag. We can adjust and introduce other opinions as we iterate.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not 100% sure these skills belong here, should this remain as a meta-module that pulls in others?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Me neither, I put it here initially instead of in the git plugin to avoid bringing many of those specs which are unneeded here as a dependency. This skill has a concrete use case and as a principle seems easier to iterate with less deps.
I moved it there and added the dependency with the git plugin. I would expect eventually we enforce distribution of a .gitlint file that runs on precommit hook

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 22, 2026
enxebre and others added 7 commits June 22, 2026 21:23
On session start, installs both 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.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move address-reviews from utils into openshift-developer as the
address-review-pr skill. Fetches and addresses all PR review
comments: categorizes by priority, makes code changes, posts
replies, and pushes. Includes check_replied.py helper for
duplicate reply prevention.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix code review findings before committing: applies fixes, runs
verification, commits, and pushes. Designed to run after
/code-review:pre-commit-review to close the pre-PR author loop.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Conventional commit formatting rules: types, scopes, breaking
changes, required footers (Signed-off-by, Commit-Message-Assisted-by),
and gitlint validation. Ported from openshift/hypershift and
generalized for any OpenShift repo.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bump version to 1.3.0, document workflows (pre-PR and post-PR loops),
prerequisites, and all new skills/hooks in README.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Validate .pre-commit-config.yaml against a trusted whitelist before
installing hooks. Local hooks are always allowed. Remote repos must
be explicitly trusted, and specific hooks can be restricted per repo.

Trusted repos:
- pre-commit/pre-commit-hooks (check-merge-conflict, check-yaml,
  trailing-whitespace only)
- leaktk/gitleaks (all hooks)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…view feedback

- Move git-commit-format skill to plugins/git (keeps openshift-developer
  as a meta-module)
- Add git plugin as a dependency
- Remove --force-with-lease from address-review skills
- Document all workflow skills in README ordered by workflow stage

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@enxebre
enxebre force-pushed the port-git-hooks-to-openshift-dev branch from 38a431e to 1f39082 Compare June 22, 2026 19:24
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 22, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/openshift-developer/scripts/validate_precommit_config.py`:
- Around line 24-33: The code assumes yaml.safe_load returns a dict on line 33,
but it can return None for empty YAML files, causing an AttributeError when
calling cfg.get(). After the yaml.safe_load call that catches exceptions, add a
validation check to ensure cfg is a dict before proceeding to the
cfg.get("repos", []) call. If cfg is None or not a dict, return exit code 2 to
handle parse/shape errors consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c55c34b6-9de0-4b26-9bb9-dcbf6da69648

📥 Commits

Reviewing files that changed from the base of the PR and between b68fd61 and 1f39082.

📒 Files selected for processing (16)
  • .claude-plugin/marketplace.json
  • .skillsaw.yaml
  • docs/index.html
  • plugins/git/.claude-plugin/plugin.json
  • plugins/git/skills/git-commit-format/SKILL.md
  • plugins/openshift-developer/.claude-plugin/plugin.json
  • plugins/openshift-developer/README.md
  • plugins/openshift-developer/hooks/hooks.json
  • plugins/openshift-developer/scripts/ensure-precommit.sh
  • plugins/openshift-developer/scripts/validate_precommit_config.py
  • plugins/openshift-developer/skills/address-review-pr/SKILL.md
  • plugins/openshift-developer/skills/address-review-pr/check_replied.py
  • plugins/openshift-developer/skills/address-review-precommit/SKILL.md
  • plugins/utils/.claude-plugin/plugin.json
  • plugins/utils/README.md
  • plugins/utils/commands/address-reviews.md
💤 Files with no reviewable changes (2)
  • plugins/utils/README.md
  • plugins/utils/commands/address-reviews.md
✅ Files skipped from review due to trivial changes (8)
  • plugins/utils/.claude-plugin/plugin.json
  • plugins/openshift-developer/hooks/hooks.json
  • .skillsaw.yaml
  • plugins/git/.claude-plugin/plugin.json
  • plugins/git/skills/git-commit-format/SKILL.md
  • plugins/openshift-developer/skills/address-review-precommit/SKILL.md
  • .claude-plugin/marketplace.json
  • plugins/openshift-developer/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • plugins/openshift-developer/scripts/ensure-precommit.sh
  • plugins/openshift-developer/skills/address-review-pr/SKILL.md
  • plugins/openshift-developer/.claude-plugin/plugin.json

Comment thread plugins/openshift-developer/scripts/validate_precommit_config.py
- Narrow exception catch to OSError/YAMLError
- Handle None/non-dict YAML shapes gracefully
- Remove --force-with-lease from address-review skills

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@stbenjam

Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2026
@openshift-ci

openshift-ci Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre, stbenjam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit ddf65e9 into openshift-eng:main Jun 22, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants