docs, feat(SREP-4410, SREP-4411: Add Claude hooks, skill, agents and update docs)#257
Merged
openshift-merge-bot[bot] merged 2 commits intoMay 29, 2026
Merged
Conversation
Contributor
Author
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
devppratik
changed the title
|
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:
WalkthroughThis PR introduces comprehensive Claude Code integration infrastructure for the OCM Agent Operator. It establishes configuration files, five specialized agents for lint/test/security/docs/CI validation, pre-commit hook tooling with prek, gitleaks secret scanning, and foundational developer documentation covering contribution, development, and testing workflows. ChangesClaude Code Integration & Automation
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #257 +/- ##
=======================================
Coverage 66.04% 66.04%
=======================================
Files 23 23
Lines 1546 1546
=======================================
Hits 1021 1021
Misses 447 447
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: 13
🧹 Nitpick comments (4)
.claude/settings.json (1)
22-25: ⚡ Quick winWildcard command permissions are very permissive.
Lines 22-25 allow
Bash(grep *),Bash(find *),Bash(ls *), andBash(cat *)with wildcard arguments. This permits arbitrary file access across the entire filesystem (not just the repository), which may exceed the intended security boundary.Consider whether these patterns should be constrained to the repository:
"Bash(grep -r * .)", "Bash(find .)", "Bash(ls -la .)", "Bash(cat ./*)"Or document the rationale if unrestricted file system read access is intentional.
🤖 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/settings.json around lines 22 - 25, The wildcard Bash permissions ("Bash(grep *)", "Bash(find *)", "Bash(ls *)", "Bash(cat *)") are too permissive and allow arbitrary filesystem reads; update those entries to restrict operations to the repository (for example restrict to current directory or recurse from '.' using patterns like grep -r, find ., ls -la ., cat ./*) or alternatively add a clear comment/documentation entry next to these symbols explaining why unrestricted filesystem read access is intentional and authorized; modify the existing symbols in .claude/settings.json to the constrained forms or add the rationale documentation so reviewers can verify the scoped permission..claude/hooks/secret-scanner.sh (2)
117-117: 💤 Low valueVerify that grep error suppression doesn't hide pattern syntax issues.
Both grep invocations suppress stderr with
2>/dev/null. While this prevents noise from complex regex patterns, it also hides legitimate errors (e.g., invalid regex syntax). During development or updates toSECRET_PATTERNS, debugging becomes harder.Consider adding a debug mode that preserves stderr, or at least document this trade-off in a comment:
# Suppress grep errors for complex patterns (set SCANNER_DEBUG=1 to see errors) if echo "$SCAN_TEXT" | grep -qE "$pattern" ${SCANNER_DEBUG:+} 2>/dev/null; thenAlso applies to: 119-119
🤖 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/hooks/secret-scanner.sh at line 117, The grep calls that currently append "2>/dev/null" (see the if using echo "$SCAN_TEXT" | grep -qE "$pattern") suppress stderr and can hide invalid-regex errors; update the check to make stderr suppression conditional on a debug flag (e.g., SCANNER_DEBUG) so developers can enable errors when needed, and add a short comment documenting the trade-off; specifically, modify the grep invocation(s) that process SECRET_PATTERNS / pattern to include conditional redirection controlled by SCANNER_DEBUG (or similar) while keeping the default behavior silent.
119-119: ⚡ Quick winConsider logging when match count is truncated.
Line 119 uses
head -5to limit extracted matches to five per pattern. If a file contains more than five instances of the same secret type, the remaining matches won't be reported to the user, potentially leaving them unaware of the full scope of the issue.📊 Suggested improvement: Notify user when matches are truncated
# Extract matches - matches=$(echo "$SCAN_TEXT" | grep -oE "$pattern" 2>/dev/null | head -5) + all_matches=$(echo "$SCAN_TEXT" | grep -oE "$pattern" 2>/dev/null) + match_count=$(echo "$all_matches" | wc -l) + matches=$(echo "$all_matches" | head -5) + + if (( match_count > 5 )); then + echo " (showing 5 of $match_count matches for $secret_type)" + fiAlternatively, if the goal is to avoid overwhelming output, keep the current behavior but document it in a comment.
🤖 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/hooks/secret-scanner.sh at line 119, The extraction step that assigns matches=$(echo "$SCAN_TEXT" | grep -oE "$pattern" 2>/dev/null | head -5) silently truncates results to five items; detect and notify when truncation happens by computing the full match count (e.g., run grep -oE "$pattern" and count with wc -l or use grep -c) and compare to the five returned, then emit a clear log/warning indicating "X additional matches truncated for pattern <pattern>" when total >5, or alternatively add an inline comment near the matches assignment documenting the intentional limit; update the logic that sets matches (and any callers that expect fewer results) to ensure the warning is logged when truncation occurs..claude/hooks/pre-edit.sh (1)
76-76: ⚡ Quick winPattern matching for boilerplate/update may not work as intended.
The condition
[[ "$FILE" != boilerplate/update ]]uses exact string comparison, which will only match the literal pathboilerplate/update. If you want to exclude any files underboilerplate/update/, you should use glob or regex patterns.📝 Suggested pattern fix
If the intent is to exclude the
boilerplate/updatedirectory and its contents:-if [[ "$FILE" == boilerplate/* ]] && [[ "$FILE" != boilerplate/update ]]; then +if [[ "$FILE" == boilerplate/* ]] && [[ "$FILE" != boilerplate/update/* ]] && [[ "$FILE" != boilerplate/update ]]; thenOr more concisely using a negative pattern:
-if [[ "$FILE" == boilerplate/* ]] && [[ "$FILE" != boilerplate/update ]]; then +if [[ "$FILE" == boilerplate/* && ! "$FILE" == boilerplate/update* ]]; then🤖 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/hooks/pre-edit.sh at line 76, The current conditional uses an exact string check [[ "$FILE" != boilerplate/update ]] which only excludes the literal file path and not files under that directory; update the condition that checks FILE (the [[ "$FILE" == boilerplate/* ]] && [[ ... ]]) to use a glob/negative pattern that excludes the boilerplate/update directory and its contents (e.g., a negative glob match against boilerplate/update/* or a combined negated pattern) so any path starting with boilerplate/update/ is excluded; locate the FILE variable and the conditional containing [[ "$FILE" == boilerplate/* ]] and replace the second comparison with a glob-aware exclusion.
🤖 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/ci-agent.md:
- Around line 237-249: The markdown code fence starting with ``` that contains
the CI output (the block beginning with "CI Status: FAILING") lacks a language
tag and triggers markdownlint MD040; update the opening fence from ``` to
```text so the block is explicitly labeled (i.e., change the unlabeled ```
before "CI Status: FAILING" to ```text) and leave the closing ``` unchanged.
In @.claude/agents/docs-agent.md:
- Around line 103-109: The examples showing unlabeled code fences (the literal
blocks containing "make go-build" and the "Bad (placeholder) make <target>")
trigger MD040; replace those unlabeled fences with either escaped backticks
(e.g. \`\`\`) or add an explicit fence label such as ```text so they are
rendered as labeled code fences; update the two places referenced (the block
that contains "make go-build" and the block that contains "make <target>") to
use the labeled/escaped fence style so the markdown linter no longer reports
MD040.
In @.claude/agents/lint-agent.md:
- Around line 78-81: The unlabeled fenced code block containing the sample lint
line "[FILE:LINE] [LINTER] Issue description / Example:
pkg/handler/deployment.go:42 [govet] unreachable code" triggers MD040; fix it by
adding a language identifier to the opening fence (e.g., "```text") so the block
becomes a labeled fence, update the block around that exact sample text in
.claude/agents/lint-agent.md, and ensure the change resolves the MD040 warning
for that code sample.
In @.claude/agents/README.md:
- Around line 96-100: The fenced code blocks in .claude/agents/README.md are
missing language identifiers and trigger markdownlint MD040; update every
triple-backtick block shown (for example the blocks starting with ""Run
lint-agent to check formatting"", the numbered lists starting with "1.
lint-agent: Fix formatting and linting" and "1. security-agent (secrets, RBAC)",
plus the other similar blocks later and the example block starting with "[AGENT]
[SEVERITY] Location: Issue") by adding a language tag (use "text") after the
opening ``` so each becomes ```text; apply the same change to the other affected
ranges (104-110, 114-119, 123-129, 185-189) to resolve MD040.
In @.claude/agents/security-agent.md:
- Around line 158-162: Update the unlabeled code fence that starts with the
“[SEVERITY] [CATEGORY] Location: Issue” block in
.claude/agents/security-agent.md by adding a language tag (use "text") to the
opening triple-backtick so the fence becomes ```text; locate the block
containing the example lines "[HIGH] [SECRET] pkg/handler/auth.go:42..." and
change only the fence marker to include the language tag.
- Around line 172-176: The change incorrectly treats adding a Kubernetes
"securityContext" as a safe auto-fix; update the auto-fix logic so that only
trailing whitespace removal and YAML indentation fixes remain in the safe list
and any insertion or modification of "securityContext" is removed from
auto-application and instead flagged for manual review. Locate the auto-fix rule
definitions (e.g., safeAutoFixes, autoFixRules, or the function
isSafeAutoFix/applySafeAutoFixes) and remove "securityContext" from the
whitelist of auto-fixable transformations, add a check to escalate modifications
that touch "securityContext" to human_review, and ensure the changelog/output
explicitly lists which manifests require manual approval.
In @.claude/hooks/pre-edit.sh:
- Around line 45-47: The hook uses raw read -r response prompts which will hang
in non-interactive environments; add a helper function confirm_or_exit that
prints the prompt, checks if stdin is a TTY (using [[ -t 0 ]]) and either exits
with a clear message in non-interactive mode or reads the response and exits
unless it matches ^[Yy]$; then replace each raw prompt instance (the read -r
response blocks around lines referencing the interactive prompts) with calls to
confirm_or_exit("your prompt text") so all interactive checks use the TTY-safe
helper.
In @.claude/hooks/README.md:
- Around line 281-299: Remove the SKIP-based bypass guidance from the "Bypassing
Pre-Commit (dangerous)" section: delete the example line showing "SKIP=hook-id
git commit" and any mention that a single hook may be skipped, replace the
emergency checklist items that instruct "Skip ONLY the blocking hook:
`SKIP=hook-id git commit`" with the suggested steps ("Investigate and fix the
hook/config first", "open an issue and request reviewer approval before merge",
"Re-run full local validation and required CI checks"), and ensure the README
explicitly forbids both `git commit --no-verify` and SKIP-based bypasses in that
section.
In @.claude/settings.json:
- Line 51: The PreToolUse hook command uses unsupported interpolation like
${args.file_path}/${args.content}/${args.new_string}; update the hook command
(the "command" entry in .claude/settings.json for the PreToolUse hooks) to read
the JSON context from stdin or from the $CLAUDE_TOOL_INPUT env var and extract
tool_input.file_path, tool_input.content, and tool_input.new_string (e.g., via
jq or a small shell parser) before invoking .claude/hooks/pre-edit.sh so the
script receives the actual values; ensure you replace all occurrences of
${args.*} in those hook commands with a call that parses the JSON and passes the
extracted fields to the script.
In @.claude/skills/prow-ci.md:
- Around line 49-51: Add an explicit fence language ("text") to the unlabeled
triple-backtick code blocks that show prow URLs (the blocks containing
"https://prow.ci.openshift.org/view/gs/test-platform-results/..." and
"https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit")
so they no longer trigger markdownlint MD040; update both occurrences (the block
around the view/gs URL and the other block around the repo query URL, including
the other instance at lines ~93-95) by changing ``` to ```text.
- Around line 163-166: Replace the macOS-only examples that use the literal
commands open
"https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator" and open
"https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit&pull=<PR_NUMBER>"
with cross-platform alternatives: document using a portable single-line fallback
(e.g., xdg-open for Linux, open for macOS, start for Windows) or suggest copying
the URL into a browser; update both occurrences to show the URL plus a
recommended command pattern like a platform fallback (xdg-open || open || start)
and a plain URL option for users who prefer to paste it manually.
In @.claude/skills/README.md:
- Around line 33-36: The fenced code block in .claude/skills/README.md is
missing a language identifier (triggering markdownlint MD040); update the
opening fence for the block that contains the two quoted prow-ci example lines
so it reads ```text (instead of just ```) to explicitly mark it as plain
text—ensure the closing fence remains ``` and save the README so the linter
passes.
In `@CONTRIBUTING.md`:
- Around line 123-129: The fenced code block that documents the commit message
format is missing a language identifier; update the opening fence for that block
to include "text" (i.e., change ``` to ```text) so the example under the commit
message format (the block containing "<type>: <short summary>" and "<optional
body>") renders correctly in Markdown.
---
Nitpick comments:
In @.claude/hooks/pre-edit.sh:
- Line 76: The current conditional uses an exact string check [[ "$FILE" !=
boilerplate/update ]] which only excludes the literal file path and not files
under that directory; update the condition that checks FILE (the [[ "$FILE" ==
boilerplate/* ]] && [[ ... ]]) to use a glob/negative pattern that excludes the
boilerplate/update directory and its contents (e.g., a negative glob match
against boilerplate/update/* or a combined negated pattern) so any path starting
with boilerplate/update/ is excluded; locate the FILE variable and the
conditional containing [[ "$FILE" == boilerplate/* ]] and replace the second
comparison with a glob-aware exclusion.
In @.claude/hooks/secret-scanner.sh:
- Line 117: The grep calls that currently append "2>/dev/null" (see the if using
echo "$SCAN_TEXT" | grep -qE "$pattern") suppress stderr and can hide
invalid-regex errors; update the check to make stderr suppression conditional on
a debug flag (e.g., SCANNER_DEBUG) so developers can enable errors when needed,
and add a short comment documenting the trade-off; specifically, modify the grep
invocation(s) that process SECRET_PATTERNS / pattern to include conditional
redirection controlled by SCANNER_DEBUG (or similar) while keeping the default
behavior silent.
- Line 119: The extraction step that assigns matches=$(echo "$SCAN_TEXT" | grep
-oE "$pattern" 2>/dev/null | head -5) silently truncates results to five items;
detect and notify when truncation happens by computing the full match count
(e.g., run grep -oE "$pattern" and count with wc -l or use grep -c) and compare
to the five returned, then emit a clear log/warning indicating "X additional
matches truncated for pattern <pattern>" when total >5, or alternatively add an
inline comment near the matches assignment documenting the intentional limit;
update the logic that sets matches (and any callers that expect fewer results)
to ensure the warning is logged when truncation occurs.
In @.claude/settings.json:
- Around line 22-25: The wildcard Bash permissions ("Bash(grep *)", "Bash(find
*)", "Bash(ls *)", "Bash(cat *)") are too permissive and allow arbitrary
filesystem reads; update those entries to restrict operations to the repository
(for example restrict to current directory or recurse from '.' using patterns
like grep -r, find ., ls -la ., cat ./*) or alternatively add a clear
comment/documentation entry next to these symbols explaining why unrestricted
filesystem read access is intentional and authorized; modify the existing
symbols in .claude/settings.json to the constrained forms or add the rationale
documentation so reviewers can verify the scoped permission.
🪄 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: 51d85068-4863-4322-9655-fb6c30a4016c
📒 Files selected for processing (19)
.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/secret-scanner.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci.md.gitleaks.toml.pre-commit-config.yamlCLAUDE.mdCONTRIBUTING.mdDEVELOPMENT.mdTESTING.md
Update docs & claude settings
devppratik
force-pushed
the
claude-and-docs
branch
from
8e8d29b to
92aa017
Compare
devppratik
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (6)
CONTRIBUTING.md (1)
123-129:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifier to fenced code block.
The commit message format example is missing a language identifier (triggers markdownlint MD040).
Proposed fix
-``` +```text <type>: <short summary> <optional body>🤖 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 `@CONTRIBUTING.md` around lines 123 - 129, Update the fenced code block example in CONTRIBUTING.md to include a language identifier by changing the opening ``` to ```text so the commit message format snippet (the block showing "<type>: <short summary>") is fenced as ```text ... ``` to satisfy markdownlint MD040..claude/skills/prow-ci/SKILL.md (3)
49-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifier to fenced code block.
The code block showing the Prow URL pattern is missing a language identifier (triggers markdownlint MD040).
Proposed fix
-``` +```text https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_ocm-agent-operator/[PR_NUMBER]/[JOB_NAME]/[JOB_ID]</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 around lines 49 - 51, The fenced code block
containing the Prow URL pattern is missing a language identifier and triggers
markdownlint MD040; update the fenced block around the URL (the triple-backtick
block showing
"https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_ocm-agent-operator/[PR_NUMBER]/[JOB_NAME]/[JOB_ID]")
to include a language identifier such as "text" (i.e., changetotext) so
the block is explicitly recognized as plain text.</details> --- `163-166`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Provide cross-platform alternatives to macOS-only `open` command.** The examples use `open` which only works on macOS. Linux users need `xdg-open` and Windows users need `start`. <details> <summary>Proposed fix</summary> ```diff # Open Prow dashboard for this repo -open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator" +# macOS: open, Linux: xdg-open, Windows: start +xdg-open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator" 2>/dev/null || open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator" # View specific PR on Prow -open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit&pull=<PR_NUMBER>" +xdg-open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit&pull=<PR_NUMBER>" 2>/dev/null || open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit&pull=<PR_NUMBER>" ``` Or simply document the URL and suggest copying it to a browser. </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 around lines 163 - 166, The examples currently call the macOS-only command open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator" and open "https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit&pull=<PR_NUMBER>"; update SKILL.md to provide cross-platform alternatives by showing xdg-open for Linux and start for Windows (or replace the open commands with a plain URL and a note telling users to copy/paste into their browser), and include a short parenthetical indicating which platform each command targets. ``` </details> --- `93-95`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add language identifier to fenced code block.** The code block showing the Prow presubmit URL is missing a language identifier (triggers markdownlint MD040). <details> <summary>Proposed fix</summary> ```diff -``` +```text https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit ``` ``` </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 around lines 93 - 95, The fenced code block
that contains the Prow presubmit URL (the block showing
"https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit")
is missing a language identifier; update that fenced block in SKILL.md to
include a language tag (e.g., add "text" after the opening triple backticks) so
the block becomestext followed by the URL and closingto satisfy
markdownlint MD040.</details> </blockquote></details> <details> <summary>.claude/skills/README.md (2)</summary><blockquote> `33-36`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add language identifier to fenced code block.** The code block showing skill invocation examples is missing a language identifier (triggers markdownlint MD040). <details> <summary>Proposed fix</summary> ```diff -``` +```text "Use the prow-ci skill to investigate the failed test in PR `#123`" "Check Prow CI results for the latest build" ``` ``` </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 33 - 36, The fenced code block in
.claude/skills/README.md containing the example skill invocations (the two lines
starting with "Use the prow-ci skill..." and "Check Prow CI results...") lacks a
language identifier; update the opening fence fromtotext so the block
becomes a labeled text block to satisfy markdownlint MD040 and improve
rendering.</details> --- `66-72`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add language identifier to fenced code block.** The directory structure example is missing a language identifier (triggers markdownlint MD040). <details> <summary>Proposed fix</summary> ```diff -``` +```text .claude/skills/ ├── README.md └── skillname/ ``` </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, The fenced code block showing
the directory tree in README.md is missing a language identifier (causing
markdownlint MD040); update the block in .claude/skills/README.md that contains
the ".claude/skills/" tree so the opening fence includes a language identifier
(e.g., change "" to "text") to mark it as plain text and satisfy the
linter while preserving the existing tree content.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>CLAUDE.md (1)</summary><blockquote> `311-314`: _💤 Low value_ **Consider rephrasing to avoid adverb repetition.** The word "only" appears twice in close proximity, which static analysis flags as potentially awkward. <details> <summary>Suggested alternative</summary> ```diff - **API Types** (`api/v1alpha1/`): Data structures only - No business logic - - Only deepcopy, validation, defaulting + - Just deepcopy, validation, defaulting - OpenAPI schema markers ``` Or: ```diff - **API Types** (`api/v1alpha1/`): Data structures only - No business logic - - Only deepcopy, validation, defaulting + - Limited to deepcopy, validation, defaulting - OpenAPI schema markers ``` </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.md` around lines 311 - 314, Rephrase the "API Types (`api/v1alpha1/`)" bullet to avoid the repeated adverb "only": change the first line to a concise descriptor like "API Types (`api/v1alpha1/`): Data structures" and reword the sub-bullets so they don't begin with "Only" (e.g., "Includes deepcopy, validation, and defaulting" and "OpenAPI schema markers"); update the three sub-bullets under the API Types heading accordingly to remove duplicated "only" and improve flow. ``` </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/agents/README.md:
- Around line 227-235: Add a language identifier to the markdown code fence in
.claude/agents/README.md (the fenced block showing the agent file tree) to
satisfy markdownlint MD040; update the opening fence fromtotext so the
block starting with ".claude/agents/" is a fenced code block with a language
tag.In @.claude/hooks/pre-edit.sh:
- Around line 11-16: Normalize the incoming path in .claude/hooks/pre-edit.sh
before running guard patterns: replace the raw FILE value by converting absolute
paths to repo-relative and stripping leading ./ (e.g., compute repo root with
git rev-parse --show-toplevel and set FILE to the path relative to that root or
remove a leading "./") so patterns like vendor/... match consistently; apply the
same normalization to the other guard block referenced (the logic around lines
69-76) so both checks operate on the normalized FILE variable rather than the
original input.In @.claude/hooks/README.md:
- Around line 11-49: The fenced ASCII diagram block is missing a language
identifier which triggers markdownlint MD040; update the opening fence for the
diagram (the triple-backtick that precedes the box starting with
"┌─────────────────────────────────────┐") to include a language tag such as
text (e.g., changetotext) so the diagram block is explicitly marked as
plain text.In @.claude/hooks/secret-scanner.sh:
- Around line 124-125: SECRETS_FOUND currently appends the raw detected value
using SECRETS_FOUND+=("$secret_type: $match"); change this to never store or
print full matches — instead append the secret_type plus a masked snippet (e.g.
show only first/last N chars with asterisks or a fixed-length redacted token) so
the variable contains no full secret values; update the identical handling at
the other occurrences referenced (lines around where SECRETS_FOUND is modified,
including the 140-142 area) so all additions use the masked format rather than
"$match".- Around line 89-90: The current scanner condition uses FILE_PATH and a regex
that blanket-skips all ".md" files (the pattern "._test.go|..md" in the
conditional), which allows secrets in Markdown to bypass scanning; update the
conditional in the hook so it no longer excludes all .md files—either remove the
"..md" alternative from the regex or narrow it to an explicit safe path (e.g.,
only skip a specific docs/ directory or README.md if truly safe) while keeping
the existing exclusions for test files and fixtures; modify the check that
references FILE_PATH to use the tightened pattern so Markdown files are scanned
for secrets.In @.claude/hooks/stop-prek-validation.sh:
- Around line 2-10: Add a preflight check for the jq binary at the top of the
script before HOOK_INPUT is parsed and before any use of jq (e.g., before the
STOP_HOOK_ACTIVE extraction). Detect jq with a simple command existence test
(command -v or which) and, if missing, emit a deterministic JSON block response
indicating failure (include keys like "allowed": false and a human-readable
"message" stating jq is required) and exit non‑zero; apply the same pattern
anywhere else in the script where jq is used (places handling STOP_HOOK_ACTIVE
and the later jq usages).In @.gitleaks.toml:
- Line 29: The current gitleaks exclusion pattern '''.*.md$''' blanket-skips
all Markdown files; remove or restrict this rule so .md files are scanned again
(or limit it to safe paths such as docs/examples only). Edit the .gitleaks.toml
entry for the Markdown exclude pattern to either delete that line or replace it
with a scoped path-based pattern (e.g., restrict to a docs/examples directory)
and rely on the existing allowlist regexes to suppress known false positives.
Duplicate comments:
In @.claude/skills/prow-ci/SKILL.md:
- Around line 49-51: The fenced code block containing the Prow URL pattern is
missing a language identifier and triggers markdownlint MD040; update the fenced
block around the URL (the triple-backtick block showing
"https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_ocm-agent-operator/[PR_NUMBER]/[JOB_NAME]/[JOB_ID]")
to include a language identifier such as "text" (i.e., changetotext) so
the block is explicitly recognized as plain text.- Around line 163-166: The examples currently call the macOS-only command open
"https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator" and open
"https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit&pull=<PR_NUMBER>";
update SKILL.md to provide cross-platform alternatives by showing xdg-open for
Linux and start for Windows (or replace the open commands with a plain URL and a
note telling users to copy/paste into their browser), and include a short
parenthetical indicating which platform each command targets.- Around line 93-95: The fenced code block that contains the Prow presubmit URL
(the block showing
"https://prow.ci.openshift.org/?repo=openshift%2Focm-agent-operator&type=presubmit")
is missing a language identifier; update that fenced block in SKILL.md to
include a language tag (e.g., add "text" after the opening triple backticks) so
the block becomestext followed by the URL and closingto satisfy
markdownlint MD040.In @.claude/skills/README.md:
- Around line 33-36: The fenced code block in .claude/skills/README.md
containing the example skill invocations (the two lines starting with "Use the
prow-ci skill..." and "Check Prow CI results...") lacks a language identifier;
update the opening fence fromtotext so the block becomes a labeled text
block to satisfy markdownlint MD040 and improve rendering.- Around line 66-72: The fenced code block showing the directory tree in
README.md is missing a language identifier (causing markdownlint MD040); update
the block in .claude/skills/README.md that contains the ".claude/skills/" tree
so the opening fence includes a language identifier (e.g., change "" to "text") to mark it as plain text and satisfy the linter while preserving the
existing tree content.In
@CONTRIBUTING.md:
- Around line 123-129: Update the fenced code block example in CONTRIBUTING.md
to include a language identifier by changing the openingtotext so the
commit message format snippet (the block showing ": ") is
fenced astext ...to satisfy markdownlint MD040.
Nitpick comments:
In@CLAUDE.md:
- Around line 311-314: Rephrase the "API Types (
api/v1alpha1/)" bullet to
avoid the repeated adverb "only": change the first line to a concise descriptor
like "API Types (api/v1alpha1/): Data structures" and reword the sub-bullets
so they don't begin with "Only" (e.g., "Includes deepcopy, validation, and
defaulting" and "OpenAPI schema markers"); update the three sub-bullets under
the API Types heading accordingly to remove duplicated "only" and improve flow.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Repository YAML (base), Central YAML (inherited) **Review profile**: CHILL **Plan**: Enterprise **Run ID**: `11c8b170-adaa-4962-840d-8421f592caa6` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 8e8d29b78fdb6f11371bf8c4f0b5dd56a2cb7170 and 92aa0172315a27b9e55926876aa578c4e501861c. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `hack/ci.sh` is excluded by `!hack/**` * `hack/prek.ci.toml` is excluded by `!hack/**` </details> <details> <summary>📒 Files selected for processing (22)</summary> * `.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/secret-scanner.sh` * `.claude/hooks/stop-prek-validation.sh` * `.claude/settings.json` * `.claude/skills/README.md` * `.claude/skills/prow-ci/SKILL.md` * `.gitleaks.toml` * `.pre-commit-config.yaml` * `.prek-version` * `CLAUDE.md` * `CONTRIBUTING.md` * `DEVELOPMENT.md` * `TESTING.md` * `prek.toml` </details> <details> <summary>✅ Files skipped from review due to trivial changes (6)</summary> * .prek-version * .claude/settings.json * .claude/hooks/cleanup.sh * DEVELOPMENT.md * TESTING.md * .claude/agents/test-agent.md </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+89
to
+90
| if [[ "$FILE_PATH" =~ ^(test/|.*_test\.go|.*\.md|.*/fixtures/) ]]; then | ||
| exit 0 |
There was a problem hiding this comment.
Avoid blanket skipping Markdown from secret scanning.
Skipping all *.md files leaves a common path for accidental secret leakage unguarded.
🔧 Suggested fix
-if [[ "$FILE_PATH" =~ ^(test/|.*_test\.go|.*\.md|.*/fixtures/) ]]; then
+if [[ "$FILE_PATH" =~ ^(test/|.*_test\.go|.*/fixtures/) ]]; then
exit 0
fi📝 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
| if [[ "$FILE_PATH" =~ ^(test/|.*_test\.go|.*\.md|.*/fixtures/) ]]; then | |
| exit 0 | |
| if [[ "$FILE_PATH" =~ ^(test/|.*_test\.go|.*/fixtures/) ]]; then | |
| exit 0 | |
| fi |
🤖 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/hooks/secret-scanner.sh around lines 89 - 90, The current scanner
condition uses FILE_PATH and a regex that blanket-skips all "*.md" files (the
pattern ".*_test\.go|.*\.md" in the conditional), which allows secrets in
Markdown to bypass scanning; update the conditional in the hook so it no longer
excludes all .md files—either remove the ".*\.md" alternative from the regex or
narrow it to an explicit safe path (e.g., only skip a specific docs/ directory
or README.md if truly safe) while keeping the existing exclusions for test files
and fixtures; modify the check that references FILE_PATH to use the tightened
pattern so Markdown files are scanned for secrets.
geowa4
suggested changes
May 20, 2026
geowa4
left a comment
geowa4
left a comment
There was a problem hiding this comment.
Other than the README.md in the agents folder that I think should make a "README" agent, I'm good with seeing how these changes work.
Member
|
@devppratik is this meant for all repos or just for operator repos? I'm trying to figure out if it would apply to say golang libraries, backend APIs, frontend UI repositories... etc |
|
This adds a few hooks but seems to be only wiring one in the settings.json |
Contributor
Author
@rolandmkunkel
One active hook |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/hooks/pre-edit.sh (1)
108-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHigh-risk ClusterRole files can bypass this guard due to an overly narrow glob (
ClusterRole*.yaml).Because the script compares the full repo-relative path (
FILE) with[[ "$FILE" == $pattern ]], paths likedeploy/20_ocm-agent-operator.ClusterRole.yamldon’t start withClusterRole, so the high-risk warning/confirmation isn’t triggered for these RBAC manifests.Suggested fix
HIGH_RISK_PATTERNS=( "*/rbac.go" "*/auth*.go" "*_rbac.yaml" "*/networkpolicy*.go" - "ClusterRole*.yaml" + "*ClusterRole*.yaml" ".tekton/*.yaml" "build/Dockerfile" )🤖 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/hooks/pre-edit.sh around lines 108 - 116, The HIGH_RISK_PATTERNS entry "ClusterRole*.yaml" is too narrow and the guard compares full repo paths using [[ "$FILE" == $pattern ]], so files like deploy/20_ocm-agent-operator.ClusterRole.yaml don’t match; update the pattern in HIGH_RISK_PATTERNS to match anywhere in the path (e.g., change "ClusterRole*.yaml" to a wildcarded form like "*ClusterRole*.yaml") and ensure the comparison using [[ "$FILE" == $pattern ]] preserves globbing (keep patterns unquoted or use the proper wildcard form) so FILE is correctly matched against patterns; verify behavior of the matching code that iterates HIGH_RISK_PATTERNS and adjust quoting if necessary.
♻️ Duplicate comments (1)
.claude/hooks/stop-prek-validation.sh (1)
23-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
jqbefore first use to keep hook decisions deterministic.
jqis invoked at Line 26 before any dependency guard. Ifjqis absent, the hook can emit non-JSON failure output and mis-handle stop decisions.Suggested fix
set -uo pipefail HOOK_INPUT=$(cat) +# Ensure jq is available before parsing hook input +if ! command -v jq >/dev/null 2>&1; then + printf '%s\n' '{"decision":"block","reason":"jq is required for stop-hook decision handling. Install jq and retry."}' + exit 0 +fi + # Allow stop on retry to prevent infinite loops -STOP_HOOK_ACTIVE=$(echo "$HOOK_INPUT" | jq -r '.stop_hook_active // false') +STOP_HOOK_ACTIVE=$(jq -r '.stop_hook_active // false' <<< "$HOOK_INPUT")Also applies to: 46-57
🤖 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/hooks/stop-prek-validation.sh around lines 23 - 27, The script uses jq to parse HOOK_INPUT into STOP_HOOK_ACTIVE (variable STOP_HOOK_ACTIVE) before verifying jq exists; add an early dependency check (e.g., command -v jq or which jq) at the top of the hook and fail deterministically by printing a JSON-formatted error and exiting non-zero if jq is missing, then proceed to parse HOOK_INPUT into STOP_HOOK_ACTIVE as currently done; apply the same existence/guard check before the other jq uses referenced around the second block (lines using jq in the later 46-57 region) so all jq invocations are protected and the hook produces deterministic JSON output on error.
🤖 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 `@docs/metrics.md`:
- Around line 27-30: The fenced code example showing the metric
ocm_agent_operator_ocm_agent_resource_absent = 1 lacks a language tag and
triggers markdownlint MD040; update the fenced block around that example (the
triple-backtick block containing ocm_agent_operator_ocm_agent_resource_absent)
to include a language tag such as text (i.e., change ``` to ```text) so the
markdown linter recognizes it as a code block.
In `@test/deploy/20_ocm-agent-operator.Role.yaml`:
- Line 63: The Role rule currently uses an over-privileged wildcard verbs entry
("verbs: ['*']") for the "networkpolicies" resource; replace the wildcard with
an explicit least-privilege set required by the controller (e.g., only the verbs
the controller uses such as get, list, watch, create, update, patch, delete) and
remove any verbs not actually needed by the controller path so the Role rule for
networkpolicies grants only the minimum required actions.
---
Outside diff comments:
In @.claude/hooks/pre-edit.sh:
- Around line 108-116: The HIGH_RISK_PATTERNS entry "ClusterRole*.yaml" is too
narrow and the guard compares full repo paths using [[ "$FILE" == $pattern ]],
so files like deploy/20_ocm-agent-operator.ClusterRole.yaml don’t match; update
the pattern in HIGH_RISK_PATTERNS to match anywhere in the path (e.g., change
"ClusterRole*.yaml" to a wildcarded form like "*ClusterRole*.yaml") and ensure
the comparison using [[ "$FILE" == $pattern ]] preserves globbing (keep patterns
unquoted or use the proper wildcard form) so FILE is correctly matched against
patterns; verify behavior of the matching code that iterates HIGH_RISK_PATTERNS
and adjust quoting if necessary.
---
Duplicate comments:
In @.claude/hooks/stop-prek-validation.sh:
- Around line 23-27: The script uses jq to parse HOOK_INPUT into
STOP_HOOK_ACTIVE (variable STOP_HOOK_ACTIVE) before verifying jq exists; add an
early dependency check (e.g., command -v jq or which jq) at the top of the hook
and fail deterministically by printing a JSON-formatted error and exiting
non-zero if jq is missing, then proceed to parse HOOK_INPUT into
STOP_HOOK_ACTIVE as currently done; apply the same existence/guard check before
the other jq uses referenced around the second block (lines using jq in the
later 46-57 region) so all jq invocations are protected and the hook produces
deterministic JSON output on error.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: aa44ea5d-d66d-4078-9ad1-7b26f42973a3
⛔ Files ignored due to path filters (15)
boilerplate/openshift/golang-osd-e2e/README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-e2e/project.mkis excluded by!boilerplate/**boilerplate/openshift/golang-osd-e2e/updateis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/TEST_README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/app-sre.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/csv-generate/csv-generate.shis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/olm_pko_migration.pyis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/test_olm_pko_migration.pyis excluded by!boilerplate/**deploy_pko/.test-fixtures/config-with-proxy/Cleanup-OLM-Job.yamlis excluded by!**/.test-fixtures/**deploy_pko/.test-fixtures/config-with-proxy/Role-openshift-config.yamlis excluded by!**/.test-fixtures/**deploy_pko/.test-fixtures/config-with-proxy/Role-openshift-monitoring.yamlis excluded by!**/.test-fixtures/**deploy_pko/.test-fixtures/config-with-proxy/RoleBinding-openshift-config.yamlis excluded by!**/.test-fixtures/**deploy_pko/.test-fixtures/config-with-proxy/RoleBinding-openshift-monitoring.yamlis excluded by!**/.test-fixtures/**hack/boilerplate.go.txtis excluded by!hack/**
📒 Files selected for processing (33)
.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/hooks/README.md.claude/hooks/pre-edit.sh.claude/hooks/secret-scanner.sh.claude/hooks/stop-prek-validation.sh.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.github/PULL_REQUEST_TEMPLATE.mdCONTRIBUTING.mdMakefiledeploy/20_ocm-agent-operator.ClusterRole.yamldeploy/20_ocm-agent.Role.yamldeploy_pko/Cleanup-OLM-Job.yamldeploy_pko/Role-openshift-config.yamldeploy_pko/Role-openshift-monitoring.yamldeploy_pko/RoleBinding-openshift-config.yamldeploy_pko/RoleBinding-openshift-monitoring.yamldocs/how-to-test.mddocs/metrics.mddocs/testing.mdtest/deploy/20_oao-monitoring-manager.RoleBinding.yamltest/deploy/20_oao-openshiftconfig-reader.RoleBinding.yamltest/deploy/20_ocm-agent-operator.ClusterRole.yamltest/deploy/20_ocm-agent-operator.Role.yamltest/deploy/20_ocm-agent.Role.yamltest/deploy/60_ocmagent.ManagedFleetNotification.yamltest/deploy/60_ocmagent.ManagedNotification.yamltest/e2e/README.mdtest/e2e/ocm_agent_operator_tests.go
💤 Files with no reviewable changes (3)
- .github/PULL_REQUEST_TEMPLATE.md
- test/deploy/20_oao-openshiftconfig-reader.RoleBinding.yaml
- test/deploy/20_oao-monitoring-manager.RoleBinding.yaml
✅ Files skipped from review due to trivial changes (18)
- deploy_pko/RoleBinding-openshift-monitoring.yaml
- deploy/20_ocm-agent.Role.yaml
- deploy_pko/Role-openshift-config.yaml
- Makefile
- test/deploy/60_ocmagent.ManagedNotification.yaml
- docs/how-to-test.md
- test/deploy/20_ocm-agent.Role.yaml
- deploy_pko/Cleanup-OLM-Job.yaml
- .claude/agents/lint-agent.md
- test/e2e/ocm_agent_operator_tests.go
- docs/testing.md
- deploy/20_ocm-agent-operator.ClusterRole.yaml
- .claude/skills/prow-ci/SKILL.md
- .claude/agents/README.md
- .claude/agents/docs-agent.md
- .claude/hooks/README.md
- .claude/agents/ci-agent.md
- .claude/skills/README.md
devppratik
force-pushed
the
claude-and-docs
branch
from
0bef952 to
2fe655d
Compare
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/hooks/pre-edit.sh:
- Around line 21-24: The prefix-strip currently uses an unquoted pattern
FILE="${FILE#$REPO_ROOT/}", which can misinterpret glob-special characters in
REPO_ROOT; change the expansion to quote the prefix so it is treated literally,
i.e. use the variables FILE and REPO_ROOT in the expression with a quoted prefix
like FILE="${FILE#"$REPO_ROOT"/}" so literal matching is enforced when
converting absolute paths to repo-relative ones.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 234cadc9-9b0c-4c30-9ecd-2348ed938aaf
⛔ Files ignored due to path filters (1)
hack/prek.ci.tomlis excluded by!hack/**
📒 Files selected for processing (13)
.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/hooks/README.md.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/skills/README.md.claude/skills/prow-ci/SKILL.mdCONTRIBUTING.mddocs/metrics.mdprek.toml
✅ Files skipped from review due to trivial changes (8)
- .claude/skills/prow-ci/SKILL.md
- CONTRIBUTING.md
- .claude/skills/README.md
- .claude/agents/ci-agent.md
- .claude/agents/docs-agent.md
- .claude/agents/README.md
- .claude/agents/lint-agent.md
- .claude/hooks/README.md
devppratik
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@devppratik: This pull request references SREP-4410 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 epic to target the "5.0.0" version, but no target version was set. This pull request references SREP-4411 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 epic 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. |
- Add markdown language tags (MD040) to all documentation - Optimize stop hook to run only when changes exist or CLAUDE_LINT_ON_STOP=true - Add TTY checks to pre-edit.sh for non-interactive safety - Fix ClusterRole pattern matching in pre-edit.sh - Add jq dependency check to stop-prek-validation.sh - Remove secret-scanner.sh (superseded by prek gitleaks) - Update hooks README to forbid bypass methods - Replace macOS-only commands with cross-platform alternatives - Remove auto-fix for securityContext (requires manual review) Configure prek exclusions for boilerplate-managed files: - Exclude boilerplate/ directory - Exclude test/e2e/ (managed by golang-osd-e2e/update) - Exclude .pre-commit-config.yaml (managed by golang-osd-operator/update) These exclusions prevent CI validate failures and avoid conflicts with upstream boilerplate updates. fix coderabbit changes
devppratik
force-pushed
the
claude-and-docs
branch
from
2fe655d to
e6ecebc
Compare
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/hooks/pre-edit.sh:
- Line 113: The current filename check uses a case-sensitive glob
"*ClusterRole*.yaml" which misses lowercase variants like "clusterrole*.yaml";
update the pre-edit hook so the high-risk pattern is matched
case-insensitively—either enable and later disable shell nocaseglob around the
globbing or replace the literal pattern with a case-insensitive alternative
(e.g., use character classes or normalize names to lowercase before matching)
where "*ClusterRole*.yaml" is used in the script so both "ClusterRole" and
"clusterrole" forms trigger the warning 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: a6b8dc52-5712-4660-8532-72d349c8e426
⛔ Files ignored due to path filters (1)
hack/prek.ci.tomlis excluded by!hack/**
📒 Files selected for processing (14)
.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/hooks/README.md.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.gitleaks.tomlCONTRIBUTING.mddocs/metrics.mdprek.toml
💤 Files with no reviewable changes (1)
- .gitleaks.toml
✅ Files skipped from review due to trivial changes (10)
- CONTRIBUTING.md
- docs/metrics.md
- prek.toml
- .claude/skills/prow-ci/SKILL.md
- .claude/agents/ci-agent.md
- .claude/agents/README.md
- .claude/agents/docs-agent.md
- .claude/agents/lint-agent.md
- .claude/skills/README.md
- .claude/hooks/README.md
| "*/auth*.go" | ||
| "*_rbac.yaml" | ||
| "*/networkpolicy*.go" | ||
| "*ClusterRole*.yaml" |
There was a problem hiding this comment.
Match ClusterRole filenames case-insensitively in high-risk patterns.
Line 113 only matches *ClusterRole*.yaml. On case-sensitive filesystems, lowercase names (for example clusterrole*.yaml) bypass this warning path.
Suggested patch
HIGH_RISK_PATTERNS=(
"*/rbac.go"
"*/auth*.go"
"*_rbac.yaml"
"*/networkpolicy*.go"
"*ClusterRole*.yaml"
+ "*clusterrole*.yaml"
+ "*cluster-role*.yaml"
".tekton/*.yaml"
"build/Dockerfile"
)🤖 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/hooks/pre-edit.sh at line 113, The current filename check uses a
case-sensitive glob "*ClusterRole*.yaml" which misses lowercase variants like
"clusterrole*.yaml"; update the pre-edit hook so the high-risk pattern is
matched case-insensitively—either enable and later disable shell nocaseglob
around the globbing or replace the literal pattern with a case-insensitive
alternative (e.g., use character classes or normalize names to lowercase before
matching) where "*ClusterRole*.yaml" is used in the script so both "ClusterRole"
and "clusterrole" forms trigger the warning consistently.
|
@devppratik Just to get some thoughts. Can't we move some of the common |
Comment on lines
+14
to
+19
| ### Primary Tasks | ||
| - Scan for hardcoded secrets and credentials | ||
| - Validate RBAC configurations (no wildcards) | ||
| - Check for insecure patterns in code | ||
| - Detect dangerous operations | ||
| - Enforce security policies |
There was a problem hiding this comment.
Any chance to reuse the rosa-claude-plugin/application-security skill instead of a custom agent?
https://github.com/openshift-online/rosa-claude-plugins/blob/main/security/skills/adversary/references/application-security.md
Contributor
Author
There was a problem hiding this comment.
Will take a look if it can be reused. However there are specific tools which the skills + pre-commit hooks uses so it makes easier for both human and AI usage
|
|
||
| ```bash | ||
| # Invoke the skill | ||
| /prow-ci |
There was a problem hiding this comment.
Can we reuse /rosa-ci-skills ?
https://github.com/openshift-online/rosa-claude-plugins/blob/main/rosa-ci-skills
Contributor
Author
There was a problem hiding this comment.
Both have different use cases. This skill is more of debugging the failures for the CI on the MRs. Sharing how it looks
Contributor
Author
|
\test validate |
Contributor
Author
|
/test validate |
Contributor
Author
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/lgtm |
Contributor
|
/lgtm |
openshift-ci
Bot
added
the
approved
Contributor
|
/approve |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: devppratik, Tafhim, tkong-redhat The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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 was referenced May 29, 2026
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
9 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?
Which Jira/Github issue(s) this PR fixes?
Fixes #SREP-4410, SREP-4411
Special notes for your reviewer:
Pre-checks (if applicable):
make generatecommand locally to validate code changesSummary by CodeRabbit
Documentation
Chores