docs, feat(SREP-4460, SREP-4926: Add Standardized Claude hooks, skill, agents. Update standardised docs)#329
Open
devppratik wants to merge 7 commits into
Open
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@devppratik: This pull request references SREP-4460 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. This pull request references SREP-4926 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Claude agents/skills documentation, Prek hook configs and scripts (pre-edit/stop/cleanup), gitleaks config and Prek pin, expanded Claude settings, CI skill utilities for Prow artifacts, and updated CONTRIBUTING/DEVELOPMENT/TESTING guides. ChangesClaude AI Framework and Developer Tooling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #329 +/- ##
=======================================
Coverage 58.34% 58.34%
=======================================
Files 26 26
Lines 1942 1942
=======================================
Hits 1133 1133
Misses 731 731
Partials 78 78 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (1)
.claude/agents/ci-agent.md (1)
149-150: ⚡ Quick winReplace placeholder version diff with a real CI source comparison.
This compares pre-commit revisions against a static placeholder, so it can’t detect drift. Point the right-hand side to the actual CI source (or remove this check to avoid false confidence).
Suggested fix
-# Compare pre-commit and CI tools -diff <(grep "rev:" .pre-commit-config.yaml) <(echo "# CI versions from boilerplate") +# Compare pre-commit tool revisions with boilerplate-pinned revisions (example path) +# Adjust the CI file path to the actual mirrored pipeline definition in this repo. +diff \ + <(grep "rev:" .pre-commit-config.yaml | sort) \ + <(grep -E "golangci-lint|gitleaks|rev:" .tekton/*.yaml | sort)🤖 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 @.claude/agents/ci-agent.md around lines 149 - 150, The current diff compares "rev:" from .pre-commit-config.yaml to a static placeholder via diff <(grep "rev:" .pre-commit-config.yaml) <(echo "# CI versions from boilerplate"); update this to compare against the real CI source or remove the check: replace the right-hand process substitution (echo "# CI versions from boilerplate") with a command that outputs the actual CI versions (e.g., cat or grep on the real CI config file) so the diff detects drift, or delete the comparison step entirely if you don't want this validation; ensure you update the command invoked in the script that runs this diff so it references the actual CI file instead of the placeholder.
🤖 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 @.claude/agents/docs-agent.md:
- Around line 179-180: The current pipeline that starts with "grep '```bash'
*.md | grep 'make '" wrongly filters to fence markers and never captures actual
make commands; replace that pipeline (the line containing the grep of fenced
bash blocks) with a command that recursively searches Markdown files for literal
"make <target>" occurrences, extracts the second token (the target name) and
then sorts unique results — i.e., use a recursive/whole-match grep (restricting
to *.md), then pull the second field (awk or cut) and pipe to sort -u so make
targets are correctly extracted.
- Line 188: The fenced output block currently ends with a plain triple-backtick
and lacks a language tag; update the fenced block in
.claude/agents/docs-agent.md (the Markdown fenced output block containing the
changelog snippet) to include a language identifier such as "text" or "diff"
(e.g., change ``` to ```text) so the block declares its language for markdown
lint compliance and consistent rendering.
In @.claude/agents/lint-agent.md:
- Around line 23-24: Replace the invalid `go fmt -l .` invocations with `gofmt
-l .` in the markdown (specifically change the lines that read "Run `go fmt -l
.`" to "Run `gofmt -l .`") and update the repeated instance later in the file
(the same "Run `go fmt -l .`" occurrence around the second block) so both
occurrences use `gofmt -l .`; leave the `go fmt ./...` auto-fix line unchanged.
In @.claude/agents/test-agent.md:
- Around line 31-39: The script currently pipes CHANGED_FILES into xargs to
compute PACKAGES and will misbehave or error when CHANGED_FILES is empty; add an
early guard after computing CHANGED_FILES (and before using xargs dirname) to
check if CHANGED_FILES is empty and exit successfully (or skip tests) to avoid
calling xargs with no input; update the block that sets PACKAGES and the
subsequent for pkg loop to be skipped when CHANGED_FILES is empty so the go test
invocation only runs when there are changed .go files.
In @.claude/hooks/README.md:
- Around line 182-183: Update the README doc text to match the actual stop hook
implementation: replace the stated command `prek run --all-files` with the real
invocation used in .claude/hooks/stop-prek-validation.sh (`prek run --config
hack/prek.ci.toml`) and describe that the script runs the command conditionally
(only when relevant files are present/when the environment detects CI),
mirroring the script's behavior rather than the old blanket `--all-files`
description.
- Line 3: The README line currently reads "OCM Agent Operator" but should use
this repository's actual project name ("GCP Project Operator"); update the text
on that line in .claude/hooks/README.md to replace "OCM Agent Operator" with the
correct project name so wording is consistent across docs and contributors
aren't confused.
In @.claude/hooks/stop-prek-validation.sh:
- Around line 69-72: The current PREK invocation in
.claude/hooks/stop-prek-validation.sh stores output in PREK_OUTPUT and exit code
in PREK_EXIT but calls `prek run --config hack/prek.ci.toml` which only checks
the default (staged) scope and thus misses unstaged/untracked edits; update the
PREK command to include all working-tree files (e.g., add prek's --all-files
flag) or build an explicit file list from git (git status/ls-files + untracked)
and pass those paths to prek so unstaged/untracked changes are validated, and
keep using PREK_OUTPUT and PREK_EXIT for reporting/exit behavior.
In @.claude/settings.json:
- Around line 37-39: Replace the overly specific deny entries "Bash(git commit
--no-verify)", "Bash(git push --force origin master)" and "Bash(git push --force
origin main)" with broader wildcard-deny patterns to prevent bypassing via
argument order or alternate refs; update the deny rules to match any git command
using --no-verify (e.g., use a pattern like Bash(git * --no-verify *)) and any
force-push invocation (e.g., Bash(git * push * --force *)) so that
functions/matches around those entries will catch all variations instead of only
the exact strings.
In @.claude/skills/README.md:
- Around line 66-72: The README directory-structure code fence is missing a
language tag which triggers MD040; edit the fenced block in
.claude/skills/README.md (the directory-structure block that starts with ``` )
and change it to declare a language such as text (i.e., use ```text) so the
block becomes a labeled text code block while keeping the same contents.
- Line 3: Update the README intro that currently reads "Reusable workflow skills
for OCM Agent Operator development." to reference the correct repository name;
replace "OCM Agent Operator" with "gcp-project-operator" (or an appropriate
description for gcp-project-operator) so the opening line accurately describes
this repo; modify the string in .claude/skills/README.md (the intro sentence
containing "OCM Agent Operator") accordingly.
In @.gitleaks.toml:
- Around line 25-32: The global allowlist in .gitleaks.toml currently contains
the broad pattern '.*_test\.go' inside the paths = [...] array which causes all
Go test files to be skipped; remove that pattern from the paths array and keep
only the intended specific exclusions (for example the existing
'''test/fixtures/.*''', '''test/deploy/.*''', '''boilerplate/.*''',
'''vendor/.*''', '''zz_generated\..*\.go''') so gitleaks will scan *_test.go
files; update the paths array in .gitleaks.toml accordingly and run a quick
gitleaks scan to verify tests are no longer globally excluded.
In `@CONTRIBUTING.md`:
- Around line 1-3: Update the heading and intro to use the canonical product
name and remove the duplicated word: change the heading text "# Contributing to
Gcp Project" to "# Contributing to GCP Project Operator" and edit the intro line
"Thank you for your interest in contributing to the Gcp Project project." to
"Thank you for your interest in contributing to GCP Project Operator." Ensure
both occurrences use the exact same capitalization and phrasing "GCP Project
Operator".
In `@hack/ci.sh`:
- Line 9: The prek invocation uses a relative path "hack/prek.ci.toml" which
breaks when the script is run from a non-root working directory; change the
script to resolve the config file path relative to the script location (e.g.,
compute the script directory/REPO_ROOT and construct an absolute path to
hack/prek.ci.toml) and use that absolute path in the "prek run --config ...
--all-files" call so config resolution is independent of the caller's cwd.
In `@TESTING.md`:
- Around line 266-275: The TESTING.md section is contradictory about whether the
pre-commit hook id "go-test" (entry: make go-test) runs automatically; either
remove the YAML snippet and clearly state that "go-test" is not enabled and
contributors must run "make go-test" manually, or add the hook to the
repository's pre-commit configuration and update the text to say it runs
automatically; update the lines that mention the pre-commit behavior and the
"go-test" id and the "make go-test" command so they consistently reflect the
single correct workflow.
---
Nitpick comments:
In @.claude/agents/ci-agent.md:
- Around line 149-150: The current diff compares "rev:" from
.pre-commit-config.yaml to a static placeholder via diff <(grep "rev:"
.pre-commit-config.yaml) <(echo "# CI versions from boilerplate"); update this
to compare against the real CI source or remove the check: replace the
right-hand process substitution (echo "# CI versions from boilerplate") with a
command that outputs the actual CI versions (e.g., cat or grep on the real CI
config file) so the diff detects drift, or delete the comparison step entirely
if you don't want this validation; ensure you update the command invoked in the
script that runs this diff so it references the actual CI file instead of the
placeholder.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ee0c90e8-a1b5-4e59-a528-811ac93235ed
📒 Files selected for processing (21)
.claude/agents/README.md.claude/agents/ci-agent.md.claude/agents/docs-agent.md.claude/agents/lint-agent.md.claude/agents/security-agent.md.claude/agents/test-agent.md.claude/hooks/README.md.claude/hooks/cleanup.sh.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.gitleaks.toml.prek-versionCONTRIBUTING.mdDEVELOPMENT.mdTESTING.mdhack/ci.shhack/prek.ci.tomlprek.toml
devppratik
force-pushed
the
add-claude-and-docs
branch
from
cba58f6 to
d0c98c0
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: devppratik The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
devppratik
force-pushed
the
add-claude-and-docs
branch
from
d0c98c0 to
baf3bc1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.claude/agents/README.md:
- Line 3: Update the opening sentence in .claude/agents/README.md by replacing
the incorrect phrase "this operator development workflows" with the
grammatically correct "this operator's development workflows" (or "the
operator's development workflows") so the description reads naturally; edit the
top line of the README to use the possessive form.
In @.claude/skills/prow-ci/SKILL.md:
- Line 7: Update the heading text to use the standard acronym casing: replace
"Prow CI Access for Gcp Project" with "Prow CI Access for GCP Project" so the
acronym GCP matches repository/product naming conventions; ensure any other
occurrences of "Gcp" in SKILL.md are similarly corrected.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 692c0833-5cdf-446c-b7fb-e8b119d55e5e
📒 Files selected for processing (21)
.claude/agents/README.md.claude/agents/ci-agent.md.claude/agents/docs-agent.md.claude/agents/lint-agent.md.claude/agents/security-agent.md.claude/agents/test-agent.md.claude/hooks/README.md.claude/hooks/cleanup.sh.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.gitleaks.toml.prek-versionCONTRIBUTING.mdDEVELOPMENT.mdTESTING.mdhack/ci.shhack/prek.ci.tomlprek.toml
✅ Files skipped from review due to trivial changes (8)
- .prek-version
- .claude/agents/test-agent.md
- TESTING.md
- .claude/agents/lint-agent.md
- CONTRIBUTING.md
- DEVELOPMENT.md
- .claude/agents/ci-agent.md
- .claude/agents/security-agent.md
🚧 Files skipped from review as they are similar to previous changes (6)
- hack/ci.sh
- .claude/hooks/cleanup.sh
- prek.toml
- .claude/hooks/pre-edit.sh
- .gitleaks.toml
- .claude/settings.json
There was a problem hiding this comment.
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 @.claude/skills/prow-ci/SKILL.md:
- Around line 114-116: Replace the mention and command that reference pre-commit
with the repository's documented tool `prek`; specifically update the
heading/title "OR use pre-commit for comprehensive linting" and the example
command line `pre-commit run --all-files` to use `prek` (e.g., adjust the
heading text and change the command to `prek run --all-files`) so the doc
matches the repo's tooling.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c8c0923d-059e-4d5d-aeee-9979035e0543
📒 Files selected for processing (21)
.claude/agents/README.md.claude/agents/ci-agent.md.claude/agents/docs-agent.md.claude/agents/lint-agent.md.claude/agents/security-agent.md.claude/agents/test-agent.md.claude/hooks/README.md.claude/hooks/cleanup.sh.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.gitleaks.toml.prek-versionCONTRIBUTING.mdDEVELOPMENT.mdTESTING.mdhack/ci.shhack/prek.ci.tomlprek.toml
✅ Files skipped from review due to trivial changes (7)
- .claude/agents/lint-agent.md
- .claude/agents/test-agent.md
- TESTING.md
- .claude/agents/security-agent.md
- CONTRIBUTING.md
- DEVELOPMENT.md
- .claude/agents/ci-agent.md
🚧 Files skipped from review as they are similar to previous changes (8)
- hack/ci.sh
- .gitleaks.toml
- .prek-version
- hack/prek.ci.toml
- .claude/settings.json
- .claude/hooks/stop-prek-validation.sh
- prek.toml
- .claude/hooks/pre-edit.sh
devppratik
force-pushed
the
add-claude-and-docs
branch
3 times, most recently
from
3255f17 to
96b4b6c
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.claude/skills/prow-ci/analyze_failure.py:
- Around line 18-24: The code uses root.findall('.//testsuite') which skips a
top-level <testsuite> element; modify the parsing logic (after
ET.parse(xml_file) and root = tree.getroot()) to include the root when root.tag
== 'testsuite' (e.g., treat root as an additional testsuite) or use an iterator
that yields root plus found children so the loop over testsuite (the variable
named testsuite) and collection into failures won't miss single-suite JUnit
files; update any related logic that builds suite_name and iterates testcase
accordingly.
In @.claude/skills/prow-ci/fetch_prow_artifacts.py:
- Around line 111-114: The loop drops directory structure by using
os.path.basename; preserve the relative path under artifacts instead: for each
gcs_file from junit_files, compute the path segments, locate the "artifacts"
segment (e.g. parts = gcs_file.split("/"); rel_parts =
parts[parts.index("artifacts"):]), join rel_parts to form rel_path, set
local_path = os.path.join(output_dir, *rel_parts), ensure the target directory
exists (os.makedirs(os.path.dirname(local_path), exist_ok=True)), then call
download_from_gcs(f"gs://{gcs_file}", local_path). Update the block that
references junit_files, filename, local_path and download_from_gcs accordingly.
In @.claude/skills/prow-ci/SKILL.md:
- Around line 75-80: Remove the author-specific absolute path in the example and
use a repo-relative invocation instead: change the `cd
/Users/ppanda/.../.claude/skills/prow-ci` step to a relative `cd
.claude/skills/prow-ci` (or instruct users to run the script from the repository
root), and keep the `python3 fetch_prow_artifacts.py "<prow-job-url>" -o
.work/prow-artifacts` invocation and output path as-is so the example is
reproducible for other contributors.
- Line 7: The document heading currently reads "# Prow CI Analysis for Rbac
Permissions Operator" which is a copy/paste of the wrong project name; update
that heading in SKILL.md (the line starting with "# Prow CI Analysis for Rbac
Permissions Operator") to reference this repository by replacing "Rbac
Permissions Operator" with the correct repository name (e.g., "# Prow CI
Analysis for <actual-repo-name>") so the title accurately reflects the current
project.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 74d84f68-63dd-4ef4-a884-a0750b4bcb03
📒 Files selected for processing (24)
.claude/agents/README.md.claude/agents/ci-agent.md.claude/agents/docs-agent.md.claude/agents/lint-agent.md.claude/agents/security-agent.md.claude/agents/test-agent.md.claude/hooks/README.md.claude/hooks/cleanup.sh.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.claude/skills/prow-ci/analyze_failure.py.claude/skills/prow-ci/fetch_prow_artifacts.py.gitignore.gitleaks.toml.prek-versionCONTRIBUTING.mdDEVELOPMENT.mdTESTING.mdhack/ci.shhack/prek.ci.tomlprek.toml
✅ Files skipped from review due to trivial changes (11)
- .gitignore
- .prek-version
- TESTING.md
- .claude/skills/README.md
- .claude/agents/test-agent.md
- .claude/agents/security-agent.md
- .claude/agents/lint-agent.md
- .claude/agents/ci-agent.md
- .claude/hooks/README.md
- .claude/agents/README.md
- DEVELOPMENT.md
🚧 Files skipped from review as they are similar to previous changes (8)
- .claude/hooks/cleanup.sh
- hack/prek.ci.toml
- .claude/hooks/stop-prek-validation.sh
- CONTRIBUTING.md
- .claude/agents/docs-agent.md
- .gitleaks.toml
- prek.toml
- .claude/settings.json
devppratik
force-pushed
the
add-claude-and-docs
branch
from
96b4b6c to
bd5aed0
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
.claude/agents/README.md (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix possessive grammar in the opening sentence.
Line 3 should use the possessive form for natural phrasing (for example: “this operator’s development workflows”).
Suggested patch
-Specialized agents for this operator development workflows. +Specialized agents for this operator's development workflows.🤖 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 @.claude/agents/README.md at line 3, Update the opening sentence to use the possessive form by changing "this operator development workflows" to "this operator’s development workflows" (or "this operator's development workflows") in the README text so the phrase reads "Specialized agents for this operator’s development workflows.".claude/agents/docs-agent.md (2)
188-188:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced output block.
Use a language identifier on Line 188 to satisfy markdown linting and keep code blocks consistent.
Suggested patch
-``` +```text Updated: DEVELOPMENT.md - Added section on new make target: go-bench - Fixed typo in test commands - Updated Go version requirement: 1.22.7 -> 1.24.0🤖 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 @.claude/agents/docs-agent.md at line 188, The fenced code block that currently uses a bare triple backtick (``` ) should specify a language tag to satisfy markdown linting; change the opening fence from ``` to ```text so the block becomes a text-fenced block containing the patch lines (e.g., the block showing "Updated: DEVELOPMENT.md" and the following lines), leaving the closing fence unchanged.
179-179:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix make-target extraction command; current pipeline won’t capture targets.
Line 179 filters on fenced-block markers, so it won’t reliably extract real
make <target>usages from markdown content.Suggested patch
-grep '```bash' *.md | grep 'make ' | sed 's/.*make \([a-z-]*\).*/\1/' | sort -u +grep -RhoE 'make [a-zA-Z0-9_.-]+' --include="*.md" . | awk '{print $2}' | sort -u🤖 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 @.claude/agents/docs-agent.md at line 179, The current extraction pipeline (the command using "grep '```bash'" and the sed expression) only searches fenced blocks and fails to capture actual "make <target>" occurrences; replace that pipeline with a recursive search that finds all "make <target>" tokens in markdown files (use a grep -R/--include pattern to limit to *.md), then extract the target token (the second whitespace-separated field) with awk and pipe to sort -u to deduplicate; update the line that currently contains the "grep '```bash' ... sed 's/.*make \([a-z-]*\).*/\1/'" pipeline to use this new recursive-grep + awk + sort approach so targets like names with digits, dots, underscores or hyphens are captured..claude/skills/README.md (1)
66-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language tag to code fence.
The directory structure code block should specify
textto satisfy MD040.📝 Proposed fix
-``` +```text .claude/skills/ ├── README.md └── skillname/ ├── SKILL.md # Required: skill definition └── reference/ # Optional: supporting docs</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 @.claude/skills/README.md around lines 66 - 72, Update the fenced code block
in README.md that shows the directory tree (the block starting with
".claude/skills/") to include the language tag "text" on the opening fence
(changetotext) so the code block satisfies MD040; locate the block in
README.md that contains the lines ".claude/skills/", "├── README.md", "└──
skillname/" and modify its opening fence accordingly.</details> </blockquote></details> <details> <summary>.claude/skills/prow-ci/SKILL.md (2)</summary><blockquote> `80-80`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Remove author-specific absolute path.** The path `/Users/ppanda/rh-projects/ROSA-730/rbac-permissions-operator/.claude/skills/prow-ci` will not exist for other contributors. <details> <summary>📝 Proposed fix</summary> ```diff -cd /Users/ppanda/rh-projects/ROSA-730/rbac-permissions-operator/.claude/skills/prow-ci +cd .claude/skills/prow-ci python3 fetch_prow_artifacts.py "<prow-job-url>" -o .work/prow-artifacts ``` </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 @.claude/skills/prow-ci/SKILL.md at line 80, Replace the author-specific absolute path string "cd /Users/ppanda/rh-projects/ROSA-730/rbac-permissions-operator/.claude/skills/prow-ci" with a non-user-specific path; e.g., use a relative path like "cd .claude/skills/prow-ci" or derive the repo root (git rev-parse --show-toplevel) and cd into "$REPO_ROOT/.claude/skills/prow-ci" so the command works for all contributors. ``` </details> --- `220-221`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Use `prek` instead of `pre-commit`.** This repository uses `prek` as the hook manager, not `pre-commit`. <details> <summary>📝 Proposed fix</summary> ```diff -# OR use pre-commit for comprehensive linting -pre-commit run --all-files +# OR use prek for comprehensive linting +prek run --all-files ``` </details> Based on learnings: Run `pre-commit run --all-files` for formatting and linting validation <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 @.claude/skills/prow-ci/SKILL.md around lines 220 - 221, The README snippet currently instructs running "pre-commit run --all-files" but this repo uses the "prek" hook manager; update the text in the SKILL.md section that contains the command string "pre-commit run --all-files" to use "prek" instead (e.g., replace "pre-commit run --all-files" with the correct "prek" invocation) so the documented hook command matches the repository's tooling. ``` </details> </blockquote></details> <details> <summary>.claude/skills/prow-ci/analyze_failure.py (1)</summary><blockquote> `22-24`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **JUnit parser skips root-level `<testsuite>` elements.** `root.findall('.//testsuite')` does not include the root node itself, so single-suite JUnit files (where `<testsuite>` is the root element) will produce an empty failure list. This is a common shape for Prow artifacts. <details> <summary>🐛 Proposed fix</summary> ```diff - for testsuite in root.findall('.//testsuite'): + for testsuite in root.iter('testsuite'): suite_name = testsuite.get('name', 'unknown') - for testcase in testsuite.findall('.//testcase'): + for testcase in testsuite.findall('testcase'): ``` </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 @.claude/skills/prow-ci/analyze_failure.py around lines 22 - 24, The JUnit parser is skipping a root-level <testsuite> because root.findall('.//testsuite') omits the root node; update the loop that currently uses root.findall('.//testsuite') so it also includes the root when root.tag == 'testsuite' (or combine results from root.findall('testsuite') and root.findall('.//testsuite')). Make this change around the block that sets suite_name and iterates testcases (references: root, root.findall('.//testsuite'), suite_name, testcase) so single-suite JUnit files are processed correctly. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with 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.Inline comments:
In @.claude/skills/prow-ci/analyze_failure.py:
- Line 18: The XML parsing call using xml.etree.ElementTree (the line tree =
ET.parse(xml_file)) is vulnerable to XXE; replace the unsafe parser by importing
the secure defusedxml implementation (e.g. change any "import
xml.etree.ElementTree as ET" to "from defusedxml import ElementTree as ET") and
keep using ET.parse(xml_file) so all existing ET.* calls (including tree =
ET.parse) use the hardened parser; also add defusedxml to dependencies
(requirements) so the runtime has the secure library available.In @.claude/skills/prow-ci/SKILL.md:
- Line 72: Add a language tag to the code fence containing the example URL in
.claude/skills/prow-ci/SKILL.md to satisfy MD040; change the fence from triple
backticks to a fenced block liketext, so the block starting with the URL (the example code block at line with the Prow URL) becomes atext fenced
block and the closing ``` remains, ensuring the code block is explicitly labeled
as text.- Line 7: Replace every hardcoded occurrence of "rbac-permissions-operator" in
.claude/skills/prow-ci/SKILL.md with "gcp-project-operator" or refactor to use a
repo-name parameter (search for the literal "rbac-permissions-operator" used in
URLs, job names, CI paths, and secret names), change the absolute path instance
beginning with "cd /Users/…/rbac-permissions-operator/…" to a repo-relative path
(e.g., use "cd ./" or "./repo-root"), and add a language tag (e.g.,block is properly tagged; keep pre-commit mentions as-is. In `@TESTING.md`: - Around line 131-147: Remove the stray nested code fence so the example block renders correctly: delete the extra "```go" line that appears immediately before the It("should reconcile resource", func() { ... }) block in TESTING.md so there is a single opening "```go" and one closing "```" surrounding the test example; the target symbols to locate are the duplicate code fence near the It("should reconcile resource", func() { ... }) example and the closing "```" that already exists after the Expect(resource.Status.Conditions).ToNot(BeEmpty()) line. --- Duplicate comments: In @.claude/agents/docs-agent.md: - Line 188: The fenced code block that currently uses a bare triple backtick (``` ) should specify a language tag to satisfy markdown linting; change the opening fence from ``` to ```text so the block becomes a text-fenced block containing the patch lines (e.g., the block showing "Updated: DEVELOPMENT.md" and the following lines), leaving the closing fence unchanged. - Line 179: The current extraction pipeline (the command using "grep '```bash'" and the sed expression) only searches fenced blocks and fails to capture actual "make <target>" occurrences; replace that pipeline with a recursive search that finds all "make <target>" tokens in markdown files (use a grep -R/--include pattern to limit to *.md), then extract the target token (the second whitespace-separated field) with awk and pipe to sort -u to deduplicate; update the line that currently contains the "grep '```bash' ... sed 's/.*make \([a-z-]*\).*/\1/'" pipeline to use this new recursive-grep + awk + sort approach so targets like names with digits, dots, underscores or hyphens are captured. In @.claude/agents/README.md: - Line 3: Update the opening sentence to use the possessive form by changing "this operator development workflows" to "this operator’s development workflows" (or "this operator's development workflows") in the README text so the phrase reads "Specialized agents for this operator’s development workflows." In @.claude/skills/prow-ci/analyze_failure.py: - Around line 22-24: The JUnit parser is skipping a root-level <testsuite> because root.findall('.//testsuite') omits the root node; update the loop that currently uses root.findall('.//testsuite') so it also includes the root when root.tag == 'testsuite' (or combine results from root.findall('testsuite') and root.findall('.//testsuite')). Make this change around the block that sets suite_name and iterates testcases (references: root, root.findall('.//testsuite'), suite_name, testcase) so single-suite JUnit files are processed correctly. In @.claude/skills/prow-ci/SKILL.md: - Line 80: Replace the author-specific absolute path string "cd /Users/ppanda/rh-projects/ROSA-730/rbac-permissions-operator/.claude/skills/prow-ci" with a non-user-specific path; e.g., use a relative path like "cd .claude/skills/prow-ci" or derive the repo root (git rev-parse --show-toplevel) and cd into "$REPO_ROOT/.claude/skills/prow-ci" so the command works for all contributors. - Around line 220-221: The README snippet currently instructs running "pre-commit run --all-files" but this repo uses the "prek" hook manager; update the text in the SKILL.md section that contains the command string "pre-commit run --all-files" to use "prek" instead (e.g., replace "pre-commit run --all-files" with the correct "prek" invocation) so the documented hook command matches the repository's tooling. In @.claude/skills/README.md: - Around line 66-72: Update the fenced code block in README.md that shows the directory tree (the block starting with ".claude/skills/") to include the language tag "text" on the opening fence (change ``` to ```text) so the code block satisfies MD040; locate the block in README.md that contains the lines ".claude/skills/", "├── README.md", "└── skillname/" and modify its opening fence accordingly.🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID:
7f50b070-f776-4a1a-a1bf-531835e336c7📒 Files selected for processing (24)
.claude/agents/README.md.claude/agents/ci-agent.md.claude/agents/docs-agent.md.claude/agents/lint-agent.md.claude/agents/security-agent.md.claude/agents/test-agent.md.claude/hooks/README.md.claude/hooks/cleanup.sh.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.claude/skills/prow-ci/analyze_failure.py.claude/skills/prow-ci/fetch_prow_artifacts.py.gitignore.gitleaks.toml.prek-versionCONTRIBUTING.mdDEVELOPMENT.mdTESTING.mdhack/ci.shhack/prek.ci.tomlprek.toml✅ Files skipped from review due to trivial changes (7)
- .gitignore
- .prek-version
- .claude/agents/test-agent.md
- .claude/agents/lint-agent.md
- .claude/agents/security-agent.md
- .claude/agents/ci-agent.md
- DEVELOPMENT.md
🚧 Files skipped from review as they are similar to previous changes (6)
- hack/ci.sh
- .claude/hooks/cleanup.sh
- .gitleaks.toml
- hack/prek.ci.toml
- CONTRIBUTING.md
- .claude/hooks/pre-edit.sh
devppratik
force-pushed
the
add-claude-and-docs
branch
2 times, most recently
from
ec50cd9 to
58ccd9c
Compare
devppratik
force-pushed
the
add-claude-and-docs
branch
from
58ccd9c to
47171d8
Compare
- Update settings.json with prek permissions and comprehensive deny rules - Update hooks (stop-prek-validation.sh, pre-edit.sh, README.md) - Update agents with repo-specific references - Update skills with repo-specific prow-ci examples - Ensure all references are operator-specific Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
Author
|
/label tide/merge-method-squash |
openshift-ci
Bot
added
the
tide/merge-method-squash
- Make pre-edit.sh canonicalization portable across GNU/BSD/macOS - Add python fallback for path normalization - Works with non-existent files - Fix stop-prek-validation.sh to explicitly validate changed files - Prevents validation scope ambiguity - Validates staged + unstaged + untracked files - Complete prow-ci SKILL.md title with operator name Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Copy latest analyze_failure.py and fetch_prow_artifacts.py from AAO - Fix gitleaks: narrow test file allowlist to testdata dirs - Remove broad .*_test.go pattern that disabled scanning for all test files
Changes: - Added SessionStart hook to .claude/settings.json that runs session-start-prek-setup.sh - Copied session-start-prek-setup.sh hook script to .claude/hooks/ - Updated docs-agent.md to include .claude/settings.json in sync triggers The SessionStart hook ensures prek is installed and configured when Claude Code starts a session, improving developer experience. Related: CodeRabbit review feedback on hook configuration documentation
Contributor
Author
|
/test validate |
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
|
@devppratik: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
docs/feature
What this PR does / why we need it?
References
Summary by CodeRabbit