docs, feat(SREP-4460, SREP-4926: Add Standardized Claude hooks, skill, agents. Update standardised docs)#641
Conversation
|
Warning Review limit reached
More reviews will be available in 9 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (21)
WalkthroughThis PR establishes comprehensive Claude Code infrastructure for the managed-upgrade-operator repository. It introduces five specialized agents (lint, test, security, docs, ci) with documented responsibilities and workflows, implements pre-commit hooks for file-edit validation and prek-based CI checks, and provides pre-commit validation configuration via gitleaks secret scanning and code quality tools. It also adds developer-facing documentation for contributing, development workflows, and testing approaches, along with a prow-ci skill for CI result analysis. ChangesClaude Code Integration
Developer Documentation
🎯 3 (Moderate) | ⏱️ ~25 minutes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[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
changed the title
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/agents/docs-agent.md:
- Around line 188-198: The Markdown fenced block in .claude/agents/docs-agent.md
(the changelog-style block shown in the diff) is missing a language tag which
triggers MD040; update the opening fence to include a language (e.g., change ```
to ```text) so the block becomes a labeled code/fenced block; ensure the rest of
the block contents remain unchanged and the closing fence is intact.
In @.claude/hooks/stop-prek-validation.sh:
- Line 21: The script currently uses "set -uo pipefail" which omits the "-e"
flag; update the shell option invocation in the script so the options include -e
(i.e., change the "set -uo pipefail" invocation to include -e) so the hook exits
immediately on any failing command; ensure there are no duplicate or conflicting
set lines and keep the final invocation as "set -euo pipefail".
In @.claude/skills/prow-ci/SKILL.md:
- Line 138: Update the prose so the product name uses the official
capitalization: change the plaintext mention of `.github` in the sentence that
reads "Skip rules: Changes to `.tekton/`, `.github/`, `.md` files, `OWNERS`,
`LICENSE` don't trigger most jobs" to use "GitHub" (i.e., keep the `.github/`
path literal intact but replace the surrounding prose mention with "GitHub") in
SKILL.md; ensure only the human-readable product name is capitalized and path
literals/quotes are unchanged.
In @.claude/skills/README.md:
- Around line 66-72: The fenced code block in README.md (the tree listing under
.claude/skills/) lacks a language identifier and triggers markdownlint MD040;
update that fenced block (the ```...``` surrounding the .claude/skills/ tree in
README.md) to include a language tag such as "text" (e.g., change ``` to
```text) so the block is labeled and the lint error is resolved.
In @.gitleaks.toml:
- Around line 25-32: The current gitleaks allowlist includes a broad pattern
('''.*_test\.go''') that exempts all Go test files; remove that
'''.*_test\.go''' entry from .gitleaks.toml and instead keep only explicit
fixture/deploy paths (e.g., test/fixtures/.*, test/deploy/.*) and, if needed,
add narrowly scoped exceptions (specific filenames or commit hashes) for known
test fixtures; update any CI documentation to explain using targeted regexes or
commit-specific allowlists rather than a global *_test.go exemption to avoid
bypassing secret scanning for normal test code.
- Around line 12-18: Add an [extend] block to explicitly enable Gitleaks'
default rules by setting useDefault = true so the custom .gitleaks.toml doesn't
override baked-in rules; update the .gitleaks.toml (near the top, before or
after the [allowlist] block) to include an [extend] section with useDefault =
true to restore full rule coverage for scans.
In `@TESTING.md`:
- Around line 266-275: The README currently gives contradictory guidance about
the 'go-test' pre-commit hook: either remove the incorrect statement that "Tests
run automatically in pre-commit when Go files change" or remove the note saying
"This is NOT in current pre-commit config"; update the TESTING.md section to
state only the truth — if the 'go-test' hook (id: go-test) is not enabled, say
"go tests are NOT run by pre-commit; run them locally with `make go-test` before
pushing", otherwise state that pre-commit will run the 'go-test' hook and remove
the manual-run instruction; ensure the text references the 'go-test' hook id and
the Makefile target `make go-test` so contributors know how to run tests.
🪄 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: 03678a15-64a0-4e27-89f4-ecdc0316fcd6
📒 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
| # - Validates changed files only (5-10s typical) | ||
| # - Uses hack/prek.ci.toml (skips network-dependent hooks) | ||
| # | ||
| set -uo pipefail |
There was a problem hiding this comment.
Missing -e flag in error handling setup.
All other scripts in this PR use set -euo pipefail, but this one uses set -uo pipefail (missing the -e flag). The -e flag ensures the script exits immediately on command failures, which is important for validation hooks to avoid incomplete checks.
🛡️ Proposed fix
-set -uo pipefail
+set -euo pipefail📝 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.
| set -uo pipefail | |
| set -euo pipefail |
🤖 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 at line 21, The script currently uses
"set -uo pipefail" which omits the "-e" flag; update the shell option invocation
in the script so the options include -e (i.e., change the "set -uo pipefail"
invocation to include -e) so the hook exits immediately on any failing command;
ensure there are no duplicate or conflicting set lines and keep the final
invocation as "set -euo pipefail".
| - Configuration: `ci-operator/config/openshift/managed-upgrade-operator/openshift-managed-upgrade-operator-master.yaml` | ||
| - Runs: lint, test, validate, coverage, e2e-binary-build | ||
| - Uses Codecov for coverage reporting (secret: `managed-upgrade-operator-codecov-token`) | ||
| - Skip rules: Changes to `.tekton/`, `.github/`, `.md` files, `OWNERS`, `LICENSE` don't trigger most jobs |
There was a problem hiding this comment.
Use official “GitHub” capitalization in prose.
At Line 138, update .github mention in sentence text to “GitHub” for product-name consistency (keep path literals unchanged).
🧰 Tools
🪛 LanguageTool
[uncategorized] ~138-~138: The official name of this software platform is spelled with a capital “H”.
Context: ...) - Skip rules: Changes to .tekton/, .github/, .mdfiles,OWNERS, LICENSE` don...
(GITHUB)
🤖 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/skills/prow-ci/SKILL.md at line 138, Update the prose so the product
name uses the official capitalization: change the plaintext mention of `.github`
in the sentence that reads "Skip rules: Changes to `.tekton/`, `.github/`, `.md`
files, `OWNERS`, `LICENSE` don't trigger most jobs" to use "GitHub" (i.e., keep
the `.github/` path literal intact but replace the surrounding prose mention
with "GitHub") in SKILL.md; ensure only the human-readable product name is
capitalized and path literals/quotes are unchanged.
| title = "gitleaks config for OCM Agent Operator" | ||
|
|
||
| # ============================================================================= | ||
| # GLOBAL ALLOWLIST | ||
| # ============================================================================= | ||
|
|
||
| [allowlist] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For current Gitleaks config syntax, when a repository defines a custom .gitleaks.tomlwith[[rules]]but no[extend]section, are the default rules still loaded automatically, or must the config set[extend] useDefault = true (or equivalent) to inherit them?
💡 Result:
When you provide a custom.gitleaks.toml configuration file, the default (baked-in) rules are not loaded automatically [1][2]. Gitleaks treats a custom configuration as a full replacement for the default ruleset unless you explicitly configure it to extend the default [1][2]. To inherit the default rules while adding your own, you must include an [extend] section in your.gitleaks.toml file and set useDefault = true [1][2][3]. Example configuration to inherit and extend: [extend] useDefault = true [[rules]] # Your custom rules go here id = "my-custom-rule"... If you omit the [extend] section or do not set useDefault = true, Gitleaks will only use the rules explicitly defined in your custom file [1][2]. When extending, rules defined in your custom file take precedence over default rules if there are naming collisions [1][4][5].
Citations:
- 1: https://openaitx.github.io/projects/gitleaks/gitleaks/README-en.html
- 2: https://github.com/gitleaks/gitleaks
- 3: https://oneuptime.com/blog/post/2026-01-25-secret-scanning-gitleaks/view
- 4: gitleaks/gitleaks@398d0c4
- 5: Documentation for the [extend] file is wrong gitleaks/gitleaks#1384
Explicitly extend Gitleaks defaults ([extend] useDefault = true)
Custom .gitleaks.toml files don’t load the baked-in default rules automatically; without an [extend] block (e.g., useDefault = true), the scan will only run the four custom [[rules]], materially reducing coverage.
🔧 Suggested fix
title = "gitleaks config for OCM Agent Operator"
+
+[extend]
+useDefault = true
# =============================================================================
# GLOBAL ALLOWLIST🤖 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 @.gitleaks.toml around lines 12 - 18, Add an [extend] block to explicitly
enable Gitleaks' default rules by setting useDefault = true so the custom
.gitleaks.toml doesn't override baked-in rules; update the .gitleaks.toml (near
the top, before or after the [allowlist] block) to include an [extend] section
with useDefault = true to restore full rule coverage for scans.
| paths = [ | ||
| '''test/fixtures/.*''', | ||
| '''test/deploy/.*''', | ||
| '''.*_test\.go''', | ||
| '''boilerplate/.*''', | ||
| '''vendor/.*''', | ||
| '''zz_generated\..*\.go''', | ||
| ] |
There was a problem hiding this comment.
Don't exempt every Go test file from secret scanning.
Line 28 allowlists all *_test.go files, which turns off gitleaks for normal test code as well as fixtures. That makes it much easier to merge real tokens, kubeconfigs, or PEM material in tests. Prefer keeping the fixture paths and handling remaining false positives with narrow regex/commit allowlists instead.
🔧 Suggested fix
paths = [
'''test/fixtures/.*''',
'''test/deploy/.*''',
- '''.*_test\.go''',
'''boilerplate/.*''',
'''vendor/.*''',
'''zz_generated\..*\.go''',
]Based on learnings "Never commit API keys, tokens, passwords, AWS credentials, kubeconfig files, private keys, certificates, .env files with secrets, or debug statements printing sensitive data".
📝 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.
| paths = [ | |
| '''test/fixtures/.*''', | |
| '''test/deploy/.*''', | |
| '''.*_test\.go''', | |
| '''boilerplate/.*''', | |
| '''vendor/.*''', | |
| '''zz_generated\..*\.go''', | |
| ] | |
| paths = [ | |
| '''test/fixtures/.*''', | |
| '''test/deploy/.*''', | |
| '''boilerplate/.*''', | |
| '''vendor/.*''', | |
| '''zz_generated\..*\.go''', | |
| ] |
🤖 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 @.gitleaks.toml around lines 25 - 32, The current gitleaks allowlist includes
a broad pattern ('''.*_test\.go''') that exempts all Go test files; remove that
'''.*_test\.go''' entry from .gitleaks.toml and instead keep only explicit
fixture/deploy paths (e.g., test/fixtures/.*, test/deploy/.*) and, if needed,
add narrowly scoped exceptions (specific filenames or commit hashes) for known
test fixtures; update any CI documentation to explain using targeted regexes or
commit-specific allowlists rather than a global *_test.go exemption to avoid
bypassing secret scanning for normal test code.
| Tests run automatically in pre-commit when Go files change: | ||
| ```yaml | ||
| - id: go-test | ||
| entry: make go-test | ||
| files: '\.go$' | ||
| ``` | ||
|
|
||
| This is NOT in current pre-commit config (too slow for pre-commit). | ||
| Run manually before pushing: `make go-test` | ||
|
|
There was a problem hiding this comment.
Resolve contradictory pre-commit test guidance.
This section states both that go-test runs automatically in pre-commit and that it is not in the current config. Please keep only one accurate statement to avoid false assumptions in contributor 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 `@TESTING.md` around lines 266 - 275, The README currently gives contradictory
guidance about the 'go-test' pre-commit hook: either remove the incorrect
statement that "Tests run automatically in pre-commit when Go files change" or
remove the note saying "This is NOT in current pre-commit config"; update the
TESTING.md section to state only the truth — if the 'go-test' hook (id: go-test)
is not enabled, say "go tests are NOT run by pre-commit; run them locally with
`make go-test` before pushing", otherwise state that pre-commit will run the
'go-test' hook and remove the manual-run instruction; ensure the text references
the 'go-test' hook id and the Makefile target `make go-test` so contributors
know how to run tests.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #641 +/- ##
=======================================
Coverage 54.27% 54.27%
=======================================
Files 123 123
Lines 6204 6204
=======================================
Hits 3367 3367
Misses 2631 2631
Partials 206 206 🚀 New features to boost your workflow:
|
devppratik
force-pushed
the
add-claude-and-docs
branch
3 times, most recently
from
ee075a0 to
4492a9f
Compare
- Add .claude/ directory with agents, hooks, and skills - Add prek validation framework (prek.toml, hack/prek.ci.toml) - Add gitleaks secret scanning (.gitleaks.toml) - Add CONTRIBUTING.md, DEVELOPMENT.md, TESTING.md - Add CLAUDE.md (if not already present) - Add stop hook for automatic validation Based on ocm-agent-operator PR openshift#257 (SREP-4410, SREP-4411) Brings the repo up to Agentic SDLC contribution standard
devppratik
force-pushed
the
add-claude-and-docs
branch
from
4492a9f to
de1bbc7
Compare
|
@devppratik: The following test failed, say
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. |
3 participants
What type of PR is this?
docs/feature
What this PR does / why we need it?
Changes
New Infrastructure:
Validation:
Documentation:
References
Summary by CodeRabbit
Documentation
Chores