docs, feat(SREP-4460, SREP-4926: Add Standardized Claude hooks, skill, agents. Update standardised docs)#556
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. |
|
[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 |
|
Warning Review limit reached
More reviews will be available in 9 minutes and 38 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 adds a Claude developer tooling suite: agent specifications (lint/test/security/docs/ci), Claude hooks and stop validation, prek/pre-commit CI configs, gitleaks secret rules, Claude permissions/settings, skills, and developer documentation (CONTRIBUTING/DEVELOPMENT/TESTING). ChangesClaude Developer Tooling & Automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (11 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 #556 +/- ##
=======================================
Coverage 68.29% 68.29%
=======================================
Files 8 8
Lines 1088 1088
=======================================
Hits 743 743
Misses 315 315
Partials 30 30 🚀 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hack/prek.ci.toml (1)
1-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGitleaks secret scanning is missing from CI configuration.
The CI variant of the prek config (
hack/prek.ci.toml) excludes the gitleaks hook that is present in the development config (prek.tomllines 26-32). This creates a security gap where secrets could be committed and reach CI without detection during pre-commit.Gitleaks does not require internal network access and should run in CI to provide a final safety check before merge. The security-agent documentation (
.claude/agents/security-agent.mdlines 208-209) explicitly states "CI: Tekton runs gitleaks and gosec", but this CI config does not enable gitleaks.🔒 Proposed fix to add gitleaks to CI config
] +# Gitleaks secret scanning +[[repos]] +repo = "https://github.com/gitleaks/gitleaks" +rev = "v8.18.0" +hooks = [ + { id = "gitleaks", args = ["--config=.gitleaks.toml"] }, +] + # golangci-lint static analysis [[repos]]Based on learnings, never commit secrets, tokens, or credentials. CI must validate this before merge.
🤖 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 `@hack/prek.ci.toml` around lines 1 - 64, The CI prek config is missing the gitleaks pre-commit hook (id "gitleaks"), so add the same gitleaks hook entry used in the dev config into hack/prek.ci.toml's hooks list: create a hook with id "gitleaks" (matching the dev/prek.toml configuration), include the same repo/rev/args/regex settings used for detection, and place it alongside the file-hygiene hooks (the first [[repos]] block) so CI runs gitleaks before merges; ensure any pass_filenames or timeout settings mirror the development config to maintain consistent behavior in CI.
🧹 Nitpick comments (2)
.claude/agents/docs-agent.md (1)
188-198: 💤 Low valueAdd language identifier to fenced code block.
The code block at line 188 lacks a language identifier. Adding
textwould improve semantic clarity and address the markdownlint warning.📝 Proposed fix
-``` +```text Updated: DEVELOPMENT.md - Added section on new make target: go-bench - Fixed typo in test commands🤖 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 around lines 188 - 198, The fenced code block using triple backticks that contains the "Updated: DEVELOPMENT.md" changelog lacks a language identifier; update the opening fence from "```" to "```text" so the block is tagged as text (use the exact identifier text) to satisfy markdownlint and improve semantic clarity..claude/skills/README.md (1)
66-72: ⚡ Quick winSpecify language for fenced code block.
The directory structure example should specify a language identifier (e.g.,
textortree) for proper rendering.📝 Proposed fix
-``` +```text .claude/skills/ ├── README.md └── skillname/🤖 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/README.md around lines 66 - 72, The fenced directory-structure block in .claude/skills/README.md lacks a language identifier; update the opening fence from ``` to a language such as ```text (or ```tree) so the tree/ASCII diagram renders correctly—modify the block that shows ".claude/skills/ ├── README.md └── skillname/ ├── SKILL.md └── reference/" to use the chosen language identifier.
🤖 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/README.md:
- Around line 180-183: Update the "Automatic Validation" docs to reflect the
stop hook's actual behavior: change the statement that the stop hook runs `prek
run --all-files` on every turn to explain that the stop hook runs conditionally
by default and invokes `prek run` with a CI config (e.g. `--config
hack/prek.ci.toml`) rather than unconditionally using `--all-files`; reference
the stop hook and the `prek run` invocation so readers understand the
conditional execution and the use of the CI config flag.
In @.claude/settings.json:
- Around line 37-42: Update .claude/settings.json to explicitly deny the
SKIP=... hook-bypass pattern by adding a deny entry alongside "Bash(git commit
--no-verify)"—for example add a rule like "Bash(SKIP=* git commit *)" (or
equivalent pattern matching used in this file) so that both env-variable
bypasses and --no-verify are blocked; modify the list that currently contains
"Bash(git commit --no-verify)" and other Bash entries to include this new deny
pattern referencing the same array where "Bash(git push --force origin master)"
and "Bash(git push --force origin main)" appear.
In @.gitleaks.toml:
- Line 28: The global allowlist entry [allowlist].paths = '.*_test\.go' is too
broad and suppresses findings in all Go tests; narrow or remove it by replacing
that pattern with a more specific path or filename regex (e.g., target only
fixture or testdata directories or explicit fixture filenames) or remove the
allowlist line entirely, ensuring you update the .gitleaks.toml entry for
[allowlist].paths to something like a scoped regex that only matches known safe
fixture test files rather than '.*_test\.go'.
- Around line 34-58: The global [allowlist] entries (the regexes array and
stopwords array) are too broad and can suppress real findings; instead, move or
duplicate only the truly benign patterns into per-rule allowlists using
targetRules, and/or tighten the patterns (e.g., anchor or scope the regexes like
the placeholder entry) so they only match intended test files; update the
regexes array and stopwords array to remove generic tokens ("test", "fake",
"dummy", "placeholder", etc.) and add scoped allowlist entries on the specific
rule definitions (using targetRules) that produced the false positives.
---
Outside diff comments:
In `@hack/prek.ci.toml`:
- Around line 1-64: The CI prek config is missing the gitleaks pre-commit hook
(id "gitleaks"), so add the same gitleaks hook entry used in the dev config into
hack/prek.ci.toml's hooks list: create a hook with id "gitleaks" (matching the
dev/prek.toml configuration), include the same repo/rev/args/regex settings used
for detection, and place it alongside the file-hygiene hooks (the first
[[repos]] block) so CI runs gitleaks before merges; ensure any pass_filenames or
timeout settings mirror the development config to maintain consistent behavior
in CI.
---
Nitpick comments:
In @.claude/agents/docs-agent.md:
- Around line 188-198: The fenced code block using triple backticks that
contains the "Updated: DEVELOPMENT.md" changelog lacks a language identifier;
update the opening fence from "```" to "```text" so the block is tagged as text
(use the exact identifier text) to satisfy markdownlint and improve semantic
clarity.
In @.claude/skills/README.md:
- Around line 66-72: The fenced directory-structure block in
.claude/skills/README.md lacks a language identifier; update the opening fence
from ``` to a language such as ```text (or ```tree) so the tree/ASCII diagram
renders correctly—modify the block that shows ".claude/skills/ ├── README.md └──
skillname/ ├── SKILL.md └── reference/" to use the chosen language identifier.
🪄 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: 99841ecf-71c2-4b18-b70e-d2b4a0f6f6da
📒 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
| ### Automatic Validation | ||
| Prek runs automatically: | ||
| - **On every turn**: Stop hook runs `prek run --all-files` | ||
| - **On commit**: Pre-commit hook runs relevant checks |
There was a problem hiding this comment.
Align automatic-validation docs with actual stop-hook behavior.
Line 182 currently says the stop hook runs prek run --all-files on every turn, but implementation is conditional by default and uses --config hack/prek.ci.toml. Please update this section to avoid misleading contributors.
📝 Suggested doc fix
### Automatic Validation
Prek runs automatically:
-- **On every turn**: Stop hook runs `prek run --all-files`
+- **On stop (default, when changes exist)**: Stop hook runs `prek run --config hack/prek.ci.toml`
+- **On every stop (strict mode)**: Set `CLAUDE_LINT_ON_STOP=true`
- **On commit**: Pre-commit hook runs relevant checks📝 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.
| ### Automatic Validation | |
| Prek runs automatically: | |
| - **On every turn**: Stop hook runs `prek run --all-files` | |
| - **On commit**: Pre-commit hook runs relevant checks | |
| ### Automatic Validation | |
| Prek runs automatically: | |
| - **On stop (default, when changes exist)**: Stop hook runs `prek run --config hack/prek.ci.toml` | |
| - **On every stop (strict mode)**: Set `CLAUDE_LINT_ON_STOP=true` | |
| - **On commit**: Pre-commit hook runs relevant checks |
🤖 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/README.md around lines 180 - 183, Update the "Automatic
Validation" docs to reflect the stop hook's actual behavior: change the
statement that the stop hook runs `prek run --all-files` on every turn to
explain that the stop hook runs conditionally by default and invokes `prek run`
with a CI config (e.g. `--config hack/prek.ci.toml`) rather than unconditionally
using `--all-files`; reference the stop hook and the `prek run` invocation so
readers understand the conditional execution and the use of the CI config flag.
| "Bash(git commit --no-verify)", | ||
| "Bash(git push --force origin master)", | ||
| "Bash(git push --force origin main)", | ||
| "Bash(rm -rf /)", | ||
| "Bash(chmod 777 *)" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Forbidden bypass examples documented:"
rg -n "no-verify|SKIP=hook-id" .claude/hooks/README.md
echo
echo "Current deny entries:"
jq -r '.permissions.deny[]' .claude/settings.jsonRepository: openshift/configure-alertmanager-operator
Length of output: 335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .claude/settings.json (top) =="
wc -l .claude/settings.json
sed -n '1,120p' .claude/settings.json
echo
echo "== .claude/hooks/README.md (around bypass examples) =="
wc -l .claude/hooks/README.md
sed -n '280,340p' .claude/hooks/README.mdRepository: openshift/configure-alertmanager-operator
Length of output: 2832
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .claude/settings.json (top) =="
wc -l .claude/settings.json
sed -n '1,120p' .claude/settings.json
echo
echo "== .claude/hooks/README.md (around bypass examples) =="
wc -l .claude/hooks/README.md
sed -n '280,340p' .claude/hooks/README.mdRepository: openshift/configure-alertmanager-operator
Length of output: 2832
Deny explicitly documented hook-bypass via SKIP=... git commit (remove -n suggestion).
.claude/hooks/README.md forbids both git commit --no-verify and SKIP=hook-id git commit, but .claude/settings.json only denies the --no-verify form—so SKIP=... git commit can still pass the ask rule (Bash(git commit *)). Add an explicit deny for the SKIP=... git commit pattern.
🔐 Suggested deny-rule expansion
"deny": [
"Bash(git commit --no-verify)",
+ "Bash(SKIP=* git commit *)",
"Bash(git push --force origin master)",
"Bash(git push --force origin main)",
"Bash(rm -rf /)",
"Bash(chmod 777 *)"
]📝 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.
| "Bash(git commit --no-verify)", | |
| "Bash(git push --force origin master)", | |
| "Bash(git push --force origin main)", | |
| "Bash(rm -rf /)", | |
| "Bash(chmod 777 *)" | |
| ] | |
| "Bash(git commit --no-verify)", | |
| "Bash(SKIP=* git commit *)", | |
| "Bash(git push --force origin master)", | |
| "Bash(git push --force origin main)", | |
| "Bash(rm -rf /)", | |
| "Bash(chmod 777 *)" | |
| ] |
🤖 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 37 - 42, Update .claude/settings.json to
explicitly deny the SKIP=... hook-bypass pattern by adding a deny entry
alongside "Bash(git commit --no-verify)"—for example add a rule like
"Bash(SKIP=* git commit *)" (or equivalent pattern matching used in this file)
so that both env-variable bypasses and --no-verify are blocked; modify the list
that currently contains "Bash(git commit --no-verify)" and other Bash entries to
include this new deny pattern referencing the same array where "Bash(git push
--force origin master)" and "Bash(git push --force origin main)" appear.
| paths = [ | ||
| '''test/fixtures/.*''', | ||
| '''test/deploy/.*''', | ||
| '''.*_test\.go''', |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For gitleaks v8 config: does [allowlist].paths = '.*_test\.go' exclude findings from all Go test files, and is narrowing to fixture directories recommended?
💡 Result:
Yes, using [allowlists] with paths = '._test.go' in your Gitleaks configuration will effectively exclude findings from any file ending in _test.go, as Gitleaks interprets the strings in the paths array as regular expressions [1][2]. Regarding whether narrowing to fixture directories is recommended: yes, this is considered a best practice for security [3][4]. While using a broad regex like '._test.go' is technically functional, it poses a security risk because it unconditionally suppresses all findings within those files [1]. If a developer accidentally hardcodes a real secret in a test file (e.g., in a test helper or a mock configuration), Gitleaks will fail to flag it, potentially allowing the secret to be committed to your repository. To maintain a secure posture while reducing false positives, it is recommended to: 1. Target Specific Paths: Instead of excluding all test files, narrow your allowlist to specific directories known to contain intentionally non-sensitive data, such as test fixtures or mock data [4]. Example: [[allowlists]] description = "Ignore test fixtures" paths = ['''test/fixtures/.*'''] 2. Use More Precise Rules: If you have specific false positives within test files, consider using regexes or other allowlist criteria (like stopwords or commit hashes) rather than blanket file-path exclusions [1][2]. 3. Prefer Inline Suppression: For one-off false positives in test files, Gitleaks supports inline suppression comments (e.g., # gitleaks:allow), which allows you to ignore a single specific finding without disabling scanning for the entire file [5]. Always prioritize the principle of least privilege by scoping your exclusions as narrowly as possible [3][4].
Citations:
- 1: https://github.com/gitleaks/gitleaks
- 2: https://github.com/zricethezav/gitleaks/blob/master/README.md
- 3: https://developer.harness.io/docs/security-testing-orchestration/sto-techref-category/gitleaks-scanner-reference
- 4: https://oneuptime.com/blog/post/2026-01-25-secret-scanning-gitleaks/view
- 5: https://openaitx.github.io/projects/gitleaks/gitleaks/README-en.html
Avoid globally allowlisting all *_test.go files in .gitleaks.toml (line 28)
[allowlist].paths = '.*_test\.go' suppresses findings across every Go test file (not just fixtures), so real secret leaks in tests can pass unnoticed.
Suggested change
paths = [
'''test/fixtures/.*''',
'''test/deploy/.*''',
- '''.*_test\.go''',
'''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 at line 28, The global allowlist entry [allowlist].paths =
'.*_test\.go' is too broad and suppresses findings in all Go tests; narrow or
remove it by replacing that pattern with a more specific path or filename regex
(e.g., target only fixture or testdata directories or explicit fixture
filenames) or remove the allowlist line entirely, ensuring you update the
.gitleaks.toml entry for [allowlist].paths to something like a scoped regex that
only matches known safe fixture test files rather than '.*_test\.go'.
| # Allow specific test values that look like secrets but aren't | ||
| regexes = [ | ||
| '''(?i)fake[_-]?token''', | ||
| '''(?i)test[_-]?secret''', | ||
| '''(?i)example[_-]?key''', | ||
| '''(?i)dummy[_-]?password''', | ||
| '''(?i)placeholder''', | ||
| '''AKIAIOSFODNN7EXAMPLE''', # AWS example from docs | ||
| ] | ||
|
|
||
| # Specific commit hashes that were already reviewed and accepted | ||
| # Format: "commit:<hash>" | ||
| # Example: "commit:abc123def456..." | ||
| commits = [] | ||
|
|
||
| # Stopwords that appear in code but aren't secrets | ||
| stopwords = [ | ||
| "example", | ||
| "test", | ||
| "fake", | ||
| "dummy", | ||
| "placeholder", | ||
| "sample", | ||
| "mock", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In gitleaks v8, do broad global allowlist regexes/stopwords (e.g., "test", "fake", "placeholder") suppress valid detections, and is scoping them per-rule preferred?
💡 Result:
In Gitleaks v8, using broad, global allowlist regexes or stopwords (such as "test", "fake", or "placeholder") is discouraged because they apply across all detection rules and significantly increase the risk of suppressing valid secret detections [1][2]. Because global allowlists take precedence over rule-specific configurations, a poorly scoped regex can inadvertently ignore sensitive credentials that match that pattern, regardless of which rule detected them [1][3][4]. Scoping allowlists per-rule is the strongly preferred best practice for the following reasons: 1. Precision and Security: Rule-specific allowlists ensure that exclusions are targeted only to the context where they are known to be safe (e.g., specific test files or dummy value formats), preventing accidental suppression of real secrets elsewhere [2][5]. 2. Configuration Control: Since Gitleaks v8.25.0, you can explicitly use the targetRules field in your allowlist configuration to apply an exclusion to one or more specific rules, rather than applying it globally [1][3][4]. This provides a clean, maintainable way to manage false positives without broad side effects [2]. 3. Targeted Mechanisms: Gitleaks provides multiple ways to scope exceptions, including paths, specific regexes, and stopwords [1][4]. Stopwords, for instance, target only the extracted secret, making them safer than broad regexes when used correctly within the context of a specific rule [1][4]. To minimize false positives while maintaining a strong security posture, you should prefer rule-specific allowlists over global ones and use the most restrictive criteria possible (e.g., scoping by file path or specific commit) whenever feasible [2].
Citations:
- 1: https://github.com/gitleaks/gitleaks
- 2: https://deepwiki.com/gitleaks/gitleaks/4.4-allowlists-and-baselines
- 3: gitleaks/gitleaks@v8.24.3...v8.25.0
- 4: https://pkg.go.dev/github.com/zricethezav/gitleaks/v8
- 5: https://medium.com/fsmk-engineering/automated-sensitive-secrets-monitoring-with-gitleaks-and-github-actions-eb724b53547d
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
sed -n '1,120p' .gitleaks.toml
echo "----"
sed -n '120,240p' .gitleaks.tomlRepository: openshift/configure-alertmanager-operator
Length of output: 9461
Narrow the global [allowlist] regexes/stopwords to avoid suppressing real findings
In .gitleaks.toml lines 34-58, regexes/stopwords are defined under the top-level [allowlist] (global), so generic entries like (?i)placeholder and stopwords such as "test", "fake", "dummy", and "placeholder" can suppress valid detections across unrelated files/rules. Prefer scoping these to the specific rules that trigger the false positives (e.g., per-rule allowlists / targetRules), and at minimum tighten the placeholder regexes (anchor them) instead of using unbounded substring matches.
🤖 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 34 - 58, The global [allowlist] entries (the
regexes array and stopwords array) are too broad and can suppress real findings;
instead, move or duplicate only the truly benign patterns into per-rule
allowlists using targetRules, and/or tighten the patterns (e.g., anchor or scope
the regexes like the placeholder entry) so they only match intended test files;
update the regexes array and stopwords array to remove generic tokens ("test",
"fake", "dummy", "placeholder", etc.) and add scoped allowlist entries on the
specific rule definitions (using targetRules) that produced the false positives.
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
devppratik
force-pushed
the
add-claude-and-docs
branch
2 times, most recently
from
2f3f617 to
03e529d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.claude/agents/docs-agent.md (1)
44-45: ⚡ Quick winValidation examples miss
docs/**/*.mdfiles.Several commands only scan top-level
*.md, so drift underdocs/is skipped even though this file declaresdocs/*.mdin scope.Suggested doc fix
-grep '```bash' -A 10 *.md | grep '^make\|^go\|^ginkgo' +grep -r '```bash' -A 10 . --include='*.md' | grep '^make\|^go\|^ginkgo' -grep -E '```$' *.md # Code blocks without language -grep -E '\[.*\]\(\./' *.md # Relative links to check +grep -r -E '```$' . --include='*.md' # Code blocks without language +grep -r -E '\[.*\]\(\./' . --include='*.md' # Relative links to check -grep '```bash' *.md | grep 'make ' | sed 's/.*make \([a-z-]*\).*/\1/' | sort -u +grep -r '```bash' . --include='*.md' | grep 'make ' | sed 's/.*make \([a-z-]*\).*/\1/' | sort -uAlso applies to: 59-61, 179-183
🤖 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 around lines 44 - 45, Update all grep invocations that scan only top-level *.md to run recursively with an include filter for markdown files: replace occurrences of patterns like "grep '```bash' -A 10 *.md", "grep -E '```$' *.md", "grep -E '\\[.*\\]\\(\\./' *.md", and "grep '```bash' *.md | grep 'make ' ..." with their recursive equivalents using "-r . --include='*.md'". Locate these exact string patterns in the doc and change them to use grep -r with --include='*.md' so docs/**/*.md are scanned.
🤖 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 fenced code block showing the changelog snippet lacks
a language tag and triggers MD040; update the block around the changelog (the
triple-backtick fence containing "Updated: DEVELOPMENT.md" and the bullet lines)
to include a language identifier (e.g., add ```text or ```diff) so the lint rule
passes and formatting is preserved.
In @.claude/skills/README.md:
- Around line 66-72: The fenced directory-tree block in .claude/skills/README.md
is missing a language tag; update the opening fence from ``` to ```text so the
block becomes a text code block (e.g., change the triple-backticks before the
".claude/skills/" tree to "```text") to satisfy MD040 and ensure the directory
tree is treated as plain text.
---
Nitpick comments:
In @.claude/agents/docs-agent.md:
- Around line 44-45: Update all grep invocations that scan only top-level *.md
to run recursively with an include filter for markdown files: replace
occurrences of patterns like "grep '```bash' -A 10 *.md", "grep -E '```$' *.md",
"grep -E '\\[.*\\]\\(\\./' *.md", and "grep '```bash' *.md | grep 'make ' ..."
with their recursive equivalents using "-r . --include='*.md'". Locate these
exact string patterns in the doc and change them to use grep -r with
--include='*.md' so docs/**/*.md are scanned.
🪄 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: dae78451-062b-4d0d-b499-39c658395e08
📒 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)
- .prek-version
- DEVELOPMENT.md
- .claude/agents/ci-agent.md
- TESTING.md
- CONTRIBUTING.md
- .claude/agents/test-agent.md
- .claude/agents/security-agent.md
🚧 Files skipped from review as they are similar to previous changes (9)
- hack/prek.ci.toml
- hack/ci.sh
- .claude/agents/lint-agent.md
- .gitleaks.toml
- .claude/settings.json
- prek.toml
- .claude/hooks/cleanup.sh
- .claude/hooks/pre-edit.sh
- .claude/hooks/stop-prek-validation.sh
| ``` | ||
| 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 | ||
|
|
||
| Validated: | ||
| - All make targets exist and work | ||
| - All command examples tested | ||
| - Links checked | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced output block.
This code fence is missing a language identifier, which breaks your own markdown-style rule and triggers MD040.
Suggested doc fix
-```
+```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
@@
- Links checked</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 188-188: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 around lines 188 - 198, The fenced code block
showing the changelog snippet lacks a language tag and triggers MD040; update
the block around the changelog (the triple-backtick fence containing "Updated:
DEVELOPMENT.md" and the bullet lines) to include a language identifier (e.g.,
add ```text or ```diff) so the lint rule passes and formatting is preserved.
| ``` | ||
| .claude/skills/ | ||
| ├── README.md | ||
| └── skillname/ | ||
| ├── SKILL.md # Required: skill definition | ||
| └── reference/ # Optional: supporting docs | ||
| ``` |
There was a problem hiding this comment.
Add a language tag for the directory tree block.
The fenced block is missing a language specifier (MD040).
Suggested doc fix
-```
+```text
.claude/skills/
├── README.md
└── skillname/
├── SKILL.md # Required: skill definition
└── reference/ # Optional: supporting docs</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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/README.md around lines 66 - 72, The fenced directory-tree
block in .claude/skills/README.md is missing a language tag; update the opening
fence from ``` to ```text so the block becomes a text code block (e.g., change
the triple-backticks before the ".claude/skills/" tree to "```text") to satisfy
MD040 and ensure the directory tree is treated as plain text.
03e529d to
c29b7ba
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
c29b7ba to
3c5f920
Compare
|
@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. |
3 participants
What type of PR is this?
docs/feature
What this PR does / why we need it?
References
Summary by CodeRabbit