docs, feat(SREP-4460, SREP-4926: Add Standardized Claude hooks, skill, agents. Update standardised docs)#750
Conversation
|
Warning Review limit reached
More reviews will be available in 8 minutes and 52 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: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (21)
WalkthroughThis PR establishes a comprehensive Claude automation framework for the OCM Agent Operator, adding five specialized agents for linting, testing, CI validation, security scanning, and documentation maintenance. It includes lifecycle hooks to prevent unsafe edits, pre-commit configurations with secret detection, and developer guides for contribution workflows. ChangesClaude Automation & Developer Tooling
🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (12 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: 9
🧹 Nitpick comments (3)
.claude/agents/test-agent.md (1)
29-40: ⚡ Quick winClarify the git diff scope for test selection.
The test selection logic uses
git diff --name-only HEAD, which shows unstaged changes in the working directory. For a PR workflow, you likely want changed files in the branch, such asgit diff --name-only origin/main...HEADorgit diff --name-only --cachedfor staged changes.Please verify this matches the intended test selection strategy.
💡 Suggested alternative for PR-based test selection
# Changed Go files -CHANGED_FILES=$(git diff --name-only HEAD | grep "\.go$") +CHANGED_FILES=$(git diff --name-only origin/main...HEAD | grep "\.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 @.claude/agents/test-agent.md around lines 29 - 40, The git diff scope currently uses CHANGED_FILES=$(git diff --name-only HEAD) which picks up unstaged working-tree changes; update the command that sets CHANGED_FILES to use the intended PR/branch scope (e.g., compare branch against main with git diff --name-only origin/main...HEAD for PR-based changed files, or use --cached if you want staged changes) so the PACKAGES and subsequent for loop that runs go test -v ./$pkg/... only run for the correct set of changed Go files; replace the CHANGED_FILES assignment accordingly and keep the downstream PACKAGES and for pkg loop unchanged..claude/agents/docs-agent.md (1)
186-198: ⚡ Quick winAdd language tag to code block in output format example.
The docs-agent instructs others to always specify language tags for code blocks (line 98: "Use
bashfor code blocks, notshorshell"), but the output format example at lines 188-198 uses a fenced code block without a language specifier.📝 Proposed fix
## Output Format When updating docs, report: -``` +```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 186 - 198, Update the output format example's fenced code block that shows the docs update (the block containing "Updated: DEVELOPMENT.md ...") to include the language tag recommended by the docs-agent ("Use `bash` for code blocks, not `sh` or `shell`"); replace the current opening fence with one that specifies bash (e.g., change the fence token before "Updated: DEVELOPMENT.md" from ``` to ```bash) so the example follows the stated guideline..claude/skills/prow-ci/SKILL.md (1)
7-7: 💤 Low valueConsider using full repository name in title.
The title "Prow CI Access for Addon" uses the abbreviated "Addon". Consider "Prow CI Access for addon-operator" for clarity and consistency with line 3 in the frontmatter.
📝 Suggested improvement
-# Prow CI Access for Addon +# Prow CI Access for addon-operator🤖 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 7, Change the top-level title string "Prow CI Access for Addon" to use the full repository name to match the frontmatter (e.g., replace with "Prow CI Access for addon-operator"); update only the header text that currently reads "Prow CI Access for Addon" so it consistently references the full repo name used elsewhere.
🤖 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 23-29: The listed Tekton YAMLs in the ci-agent documentation are
incorrect: replace the non-existent ocm-agent-operator-* entries
(ocm-agent-operator-pull-request.yaml, ocm-agent-operator-push.yaml,
ocm-agent-operator-e2e-pull-request.yaml, ocm-agent-operator-e2e-push.yaml,
ocm-agent-operator-pko-push.yaml, ocm-agent-operator-pko-pull-request.yaml) with
the actual pipeline filenames used in the repo and remove the e2e entries;
update ci-agent.md to reference addon-operator-pull-request.yaml,
addon-operator-push.yaml, addon-operator-pko-pull-request.yaml, and
addon-operator-pko-push.yaml and adjust any validation or lookup logic that
expects the old six-file set to instead check for these four files.
In @.claude/hooks/README.md:
- Around line 181-183: The README claim that the stop hook runs "prek run
--all-files" is incorrect; update the documentation to reflect the actual stop
hook command used by the script (prek run --config hack/prek.ci.toml). Edit the
section describing the stop hook (referencing "stop hook" and the current README
lines mentioning "prek run --all-files") so it states the actual command "prek
run --config hack/prek.ci.toml" and briefly note that it uses the
hack/prek.ci.toml config rather than running all files.
In @.claude/settings.json:
- Around line 44-55: The repo only wires a Stop hook; to enforce pre-edit
blocking add a hooks.PreToolUse command hook in .claude/settings.json that
targets the file-edit tools (Edit/Write/MultiEdit) and invokes
.claude/hooks/pre-edit.sh; update the hook invocation so the target file path is
passed correctly (either by wrapping the command to forward the tool input as
the first arg or by changing pre-edit.sh to read the tool input from stdin), and
ensure the wrapper or script extracts the file path and exits non‑zero on
failure (reference hooks.PreToolUse, .claude/hooks/pre-edit.sh, and the
Edit/Write/MultiEdit tools).
In @.claude/skills/prow-ci/SKILL.md:
- Around line 69-83: Update .claude/skills/prow-ci/SKILL.md to correct the
Tekton pipeline name: replace the non-existent reference to
addon-operator-e2e-pull-request with the actual pipeline filename(s) found in
.tekton/ (e.g., addon-operator-pull-request, addon-operator-pko-pull-request,
addon-operator-push) or add a brief note pointing to where the E2E pipeline is
defined; also leave the Prow job list as-is but keep the “configured in
openshift/release” qualifier so readers know those jobs aren’t in this repo.
- Around line 84-128: The SKILL.md entry references a non-existent make target
"e2e-binary-build"; update the document to either use the actual E2E binary
build make target that exists in the repo (replace "make e2e-binary-build" with
the correct target name) or add a corresponding make rule named
"e2e-binary-build" to the repo; look for the other existing targets referenced
in the file ("go-test", "go-check", "validate", "coverage", "docker-build") to
match style and naming when choosing the correct replacement or when adding the
new rule.
In @.claude/skills/README.md:
- Line 66: Update the fenced code block in .claude/skills/README.md that shows
the directory structure to include a language specifier (e.g., change the
opening "```" to "```text") so the block for the directory tree renders and
lints correctly; locate the directory structure snippet in README.md and modify
its opening fence accordingly.
In @.gitleaks.toml:
- Around line 25-32: The gitleaks config is too permissive because the paths
array contains the pattern '.*_test\.go' which excludes all Go test files;
change the 'paths' entry by removing or narrowing that pattern and instead
whitelist only explicit test directories or file-glob anchors (e.g., limit to
specific directories like test/fixtures or test/deploy or use anchored regexes
that target only those paths) so that only intended fixture/deploy test files
are exempted; update the 'paths' array entry that currently contains
'.*_test\.go' accordingly.
In `@hack/ci.sh`:
- Line 9: The command "prek run --config hack/prek.ci.toml --all-files" relies
on the current working directory; change it to compute the repository root from
the script location and pass an absolute config path instead. In the hack/ci.sh
script, determine REPO_ROOT (e.g., from the script dirname/parent) and replace
the literal "hack/prek.ci.toml" in the "prek run --config hack/prek.ci.toml
--all-files" invocation with "$REPO_ROOT/hack/prek.ci.toml" (or equivalent
absolute path variable) so the config is resolved independent of cwd.
In `@TESTING.md`:
- Around line 264-274: Update the "Pre-commit Integration" section in TESTING.md
to remove the contradictory example and make the status explicit: start with a
sentence that tests are NOT run by pre-commit hooks (too slow), then instruct
readers to run `make go-test` manually before pushing, add a brief rationale
that the full suite takes 2+ minutes and pre-commit is reserved for fast checks,
and optionally note how to enable it locally by adding the shown go-test hook
snippet to .pre-commit-config.yaml; edit the "Pre-commit Integration" header and
body so the YAML example is presented only as an optional local enablement and
the main guidance clearly states tests are not run automatically.
---
Nitpick comments:
In @.claude/agents/docs-agent.md:
- Around line 186-198: Update the output format example's fenced code block that
shows the docs update (the block containing "Updated: DEVELOPMENT.md ...") to
include the language tag recommended by the docs-agent ("Use `bash` for code
blocks, not `sh` or `shell`"); replace the current opening fence with one that
specifies bash (e.g., change the fence token before "Updated: DEVELOPMENT.md"
from ``` to ```bash) so the example follows the stated guideline.
In @.claude/agents/test-agent.md:
- Around line 29-40: The git diff scope currently uses CHANGED_FILES=$(git diff
--name-only HEAD) which picks up unstaged working-tree changes; update the
command that sets CHANGED_FILES to use the intended PR/branch scope (e.g.,
compare branch against main with git diff --name-only origin/main...HEAD for
PR-based changed files, or use --cached if you want staged changes) so the
PACKAGES and subsequent for loop that runs go test -v ./$pkg/... only run for
the correct set of changed Go files; replace the CHANGED_FILES assignment
accordingly and keep the downstream PACKAGES and for pkg loop unchanged.
In @.claude/skills/prow-ci/SKILL.md:
- Line 7: Change the top-level title string "Prow CI Access for Addon" to use
the full repository name to match the frontmatter (e.g., replace with "Prow CI
Access for addon-operator"); update only the header text that currently reads
"Prow CI Access for Addon" so it consistently references the full repo name used
elsewhere.
🪄 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: a508032a-ab23-4583-940b-dec6cc5b1649
📒 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
| **Tekton Pipelines** (`.tekton/`): | ||
| - `ocm-agent-operator-pull-request.yaml`: PR validation | ||
| - `ocm-agent-operator-push.yaml`: Main branch builds | ||
| - `ocm-agent-operator-e2e-pull-request.yaml`: E2E tests on PR | ||
| - `ocm-agent-operator-e2e-push.yaml`: E2E tests on merge | ||
| - `ocm-agent-operator-pko-push.yaml`: PKO deployment | ||
| - `ocm-agent-operator-pko-pull-request.yaml`: PKO validation |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Tekton pipeline files exist
EXPECTED_PIPELINES=(
".tekton/ocm-agent-operator-pull-request.yaml"
".tekton/ocm-agent-operator-push.yaml"
".tekton/ocm-agent-operator-e2e-pull-request.yaml"
".tekton/ocm-agent-operator-e2e-push.yaml"
".tekton/ocm-agent-operator-pko-push.yaml"
".tekton/ocm-agent-operator-pko-pull-request.yaml"
)
echo "Checking for Tekton pipeline files..."
for pipeline in "${EXPECTED_PIPELINES[@]}"; do
if [ -f "$pipeline" ]; then
echo "✓ $pipeline"
else
echo "✗ $pipeline NOT FOUND"
fi
done
echo ""
echo "Actual Tekton files in repository:"
fd -e yaml -e yml . .tekton/Repository: openshift/addon-operator
Length of output: 632
Fix Tekton pipeline references in ci-agent.md.
The six .tekton/ocm-agent-operator-*.yaml files listed in .claude/agents/ci-agent.md (lines 23-29) do not exist in the repo. Only these .tekton/*.yaml files are present:
.tekton/addon-operator-pull-request.yaml.tekton/addon-operator-push.yaml.tekton/addon-operator-pko-pull-request.yaml.tekton/addon-operator-pko-push.yaml
Update the ci-agent documentation/validation logic to use the correct filenames (and remove/adjust the missing e2e-* entries).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/agents/ci-agent.md around lines 23 - 29, The listed Tekton YAMLs in
the ci-agent documentation are incorrect: replace the non-existent
ocm-agent-operator-* entries (ocm-agent-operator-pull-request.yaml,
ocm-agent-operator-push.yaml, ocm-agent-operator-e2e-pull-request.yaml,
ocm-agent-operator-e2e-push.yaml, ocm-agent-operator-pko-push.yaml,
ocm-agent-operator-pko-pull-request.yaml) with the actual pipeline filenames
used in the repo and remove the e2e entries; update ci-agent.md to reference
addon-operator-pull-request.yaml, addon-operator-push.yaml,
addon-operator-pko-pull-request.yaml, and addon-operator-pko-push.yaml and
adjust any validation or lookup logic that expects the old six-file set to
instead check for these four files.
| 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 stop-hook command docs with the actual script behavior.
Line 182 says the stop hook runs prek run --all-files, but the hook script executes prek run --config hack/prek.ci.toml. Please update this section so troubleshooting and expectations match reality.
🤖 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 181 - 183, The README claim that the
stop hook runs "prek run --all-files" is incorrect; update the documentation to
reflect the actual stop hook command used by the script (prek run --config
hack/prek.ci.toml). Edit the section describing the stop hook (referencing "stop
hook" and the current README lines mentioning "prek run --all-files") so it
states the actual command "prek run --config hack/prek.ci.toml" and briefly note
that it uses the hack/prek.ci.toml config rather than running all files.
| "hooks": { | ||
| "Stop": [ | ||
| { | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "bash .claude/hooks/stop-prek-validation.sh" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
# Verify current hook wiring in repository config/docs
rg -n -C2 '\"hooks\"|\"Stop\"|pre-edit|stop-prek-validation' .claude/settings.json .claude/hooks/README.mdRepository: openshift/addon-operator
Length of output: 183
🌐 Web query:
Claude Code hooks configuration: what is the correct event/key for running a command before file edits, and what is the expected JSON schema in settings.json?
💡 Result:
To run a command before file edits in Claude Code, you should use the PreToolUse hook event with a matcher targeting the appropriate tool names, such as Edit, Write, or MultiEdit [1][2][3][4]. Configuration Details: - Hook Event: PreToolUse [1][5][6] - Matcher: Use the regex "Edit|Write|MultiEdit" (or specific tools as needed) to ensure the hook triggers only when these file-modifying tools are invoked [1][2][6][4]. Expected JSON Schema in settings.json: Hooks are defined within a hooks block in your ~/.claude/settings.json (global) or.claude/settings.json (project-level) file [1][5][7]. The following structure is the standard configuration: { "$schema": "https://json.schemastore.org/claude-code-settings.json", "hooks": { "PreToolUse": [ { "matcher": "Edit|Write|MultiEdit", "hooks": [ { "type": "command", "command": "your-command-here" } ] } ] } } Key Implementation Notes: - Matchers: If you omit a matcher, the hook will fire for every tool call [1]. Using a matcher is critical for targeting specific actions like file edits [1][8]. - Input Format: When the hook executes, Claude Code passes a JSON payload to the command via stdin, containing the session ID, tool name, and tool input (including the file path) [5][3][9]. - Schema: Pointing your settings file to the official schema https://json.schemastore.org/claude-code-settings.json enables IDE autocomplete and validation for these structures [2][10][11].
Citations:
- 1: https://code.claude.com/docs/en/hooks-guide
- 2: https://dev.to/jangwook_kim_e31e7291ad98/claude-code-hooks-security-gates-for-agent-workflows-5he7
- 3: https://dev.to/rouhaiseki/i-built-two-claude-code-hooks-to-stop-it-from-leaking-my-env-files-and-wiping-my-dev-db-3a29
- 4: https://docs.gitbutler.com/features/ai-integration/claude-code-hooks
- 5: https://claudecodeguides.com/claude-code-hooks-complete-guide/
- 6: https://dev.to/shrsv/claude-code-hooks-101-turning-your-ai-coding-assistant-into-an-automated-teammate-4lee
- 7: https://code.claude.com/docs/en/hooks.md
- 8: https://claude.yourdocs.dev/docs/claude-code/hooks
- 9: https://code.claude.com/docs/en/hooks
- 10: https://code.claude.com/docs/en/settings
- 11: Link to JSON Schema for settings.json in the official docs anthropics/claude-code#11795
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Show the relevant settings.json area
echo "== .claude/settings.json (lines 30-80) =="
nl -ba .claude/settings.json | sed -n '30,80p'
# Confirm hook scripts exist
echo "== .claude/hooks directory =="
ls -la .claude/hooks || true
# Check whether pre-edit.sh is referenced anywhere
echo "== References to pre-edit.sh =="
rg -n --hidden --no-ignore-vcs "pre-edit\.sh" .claude || true
# Check hooks key presence in settings.json
echo "== Hook keys present in .claude/settings.json =="
rg -n "\"(hooks|PreToolUse|PreToolUse|Stop|matcher)\"" .claude/settings.json || trueRepository: openshift/addon-operator
Length of output: 171
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== Files under .claude =="
ls -la .claude || true
find .claude -maxdepth 3 -type f -print || true
echo "== .claude/settings.json (lines 1-120) =="
if [ -f .claude/settings.json ]; then
cat -n .claude/settings.json | sed -n '1,120p'
fi
echo "== .claude/hooks directory listing =="
ls -la .claude/hooks || true
echo "== References to pre-edit.sh in .claude =="
rg -n --hidden --no-ignore-vcs "pre-edit\.sh" .claude || true
echo "== Hook keys present in .claude/settings.json =="
if [ -f .claude/settings.json ]; then
rg -n "\"hooks\"|\"PreToolUse\"|\"Stop\"|\"matcher\"|\"type\"" .claude/settings.json || true
fiRepository: openshift/addon-operator
Length of output: 3332
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== .claude/hooks/README.md: search for PreToolUse / matcher wiring =="
rg -n "PreToolUse|matcher|Edit|Write|MultiEdit|hooks\\s*\\{|\"hooks\"|pre-edit\\.sh" .claude/hooks/README.md
echo "== .claude/hooks/README.md: lines around pre-edit.sh reference =="
nl -ba .claude/hooks/README.md | sed -n '80,160p'
echo "== .claude/hooks/README.md: lines around Implementation section =="
nl -ba .claude/hooks/README.md | sed -n '230,310p'
echo "== .claude/hooks/pre-edit.sh (head) =="
head -n 120 .claude/hooks/pre-edit.sh
echo "== .claude/hooks/pre-edit.sh (tail) =="
tail -n 120 .claude/hooks/pre-edit.shRepository: openshift/addon-operator
Length of output: 397
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== .claude/hooks/README.md: PreToolUse / matcher mentions =="
rg -n --hidden --no-ignore-vcs "PreToolUse|matcher|Edit|Write|MultiEdit|Stop\"|Stop\b|pre-edit\.sh" .claude/hooks/README.md || true
echo "== .claude/hooks/README.md: around first pre-edit section (95-140) =="
sed -n '95,140p' .claude/hooks/README.md
echo "== .claude/hooks/README.md: around Implementation section (240-290) =="
sed -n '240,290p' .claude/hooks/README.md
echo "== .claude/hooks/pre-edit.sh (head) =="
head -n 80 .claude/hooks/pre-edit.sh
echo "== .claude/hooks/pre-edit.sh (tail) =="
tail -n 80 .claude/hooks/pre-edit.sh
echo "== Repo-wide: any additional Claude settings.json / hooks config =="
find . -maxdepth 4 -name "settings.json" -path "*\.claude*" -print || true
echo "== Repo-wide: any usage of PreToolUse / pre-edit.sh wiring =="
rg -n --hidden --no-ignore-vcs "PreToolUse|\"PreToolUse\"|pre-edit\.sh" . || trueRepository: openshift/addon-operator
Length of output: 8277
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== settings.json hooks block =="
cat .claude/settings.json
echo "== hooks/README.md: top section (hook guide / examples) =="
awk 'NR>=1 && NR<=90 {print NR ":" $0}' .claude/hooks/README.md
echo "== hooks/README.md: pre-edit.sh section (look for wiring details) =="
awk 'NR>=90 && NR<=160 {print NR ":" $0}' .claude/hooks/README.md
echo "== hooks/README.md: implementation / standalone notes =="
awk 'NR>=240 && NR<=290 {print NR ":" $0}' .claude/hooks/README.md
echo "== Any mention of PreToolUse or matcher in repo besides README =="
rg -n --hidden --no-ignore-vcs "PreToolUse|matcher" .claude || trueRepository: openshift/addon-operator
Length of output: 8403
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== List top-level prek configs =="
ls -la | rg -n "prek\.toml|hack/prek\.ci\.toml" || true
find . -maxdepth 3 -type f -name "prek*.toml" -o -name "prek*.yaml" -o -name "*.toml" | head -n 50
echo "== Search for pre-edit.sh usage =="
rg -n --hidden --no-ignore-vcs "pre-edit\.sh" . || true
echo "== Search for generated-file patterns in prek configs/scripts =="
rg -n --hidden --no-ignore-vcs "zz_generated|generated/mock_|vendor/|boilerplate|ClusterRole|NetworkPolicy|_rbac\.yaml|rbac\.go|auth.*\.go|mockgen" . || true
echo "== Show prek.toml and hack/prek.ci.toml if present =="
if [ -f prek.toml ]; then echo "---- prek.toml (first 200 lines) ----"; sed -n '1,200p' prek.toml; fi
if [ -f hack/prek.ci.toml ]; then echo "---- hack/prek.ci.toml (first 200 lines) ----"; sed -n '1,200p' hack/prek.ci.toml; fi
# If configs exist under other paths, print them too
find . -type f -name "prek.toml" -o -name "prek.ci.toml" -o -name "prek.ci.*.toml" | while read -r f; do
echo "---- ${f} (first 120 lines) ----"
sed -n '1,120p' "$f"
doneRepository: openshift/addon-operator
Length of output: 50380
Wire pre-edit.sh into Claude Code “before edit” hooks (it’s not enforced right now).
.claude/settings.jsononly configureshooks.Stop(callsbash .claude/hooks/stop-prek-validation.sh);pre-edit.shisn’t wired as a Claude Code hook (repo docs list it as “standalone”).- If the goal is block-before-edit behavior, add a
hooks.PreToolUsecommand hook for the file-edit tools (e.g., Edit/Write/MultiEdit) and ensure the hook passes/extracts the target file path—pre-edit.shcurrently requires the file path as$1, while Claude command hooks typically provide tool input via stdin.
🤖 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 44 - 55, The repo only wires a Stop hook;
to enforce pre-edit blocking add a hooks.PreToolUse command hook in
.claude/settings.json that targets the file-edit tools (Edit/Write/MultiEdit)
and invokes .claude/hooks/pre-edit.sh; update the hook invocation so the target
file path is passed correctly (either by wrapping the command to forward the
tool input as the first arg or by changing pre-edit.sh to read the tool input
from stdin), and ensure the wrapper or script extracts the file path and exits
non‑zero on failure (reference hooks.PreToolUse, .claude/hooks/pre-edit.sh, and
the Edit/Write/MultiEdit tools).
| ### 4. Common Job Names | ||
|
|
||
| **Prow CI Jobs** (configured in openshift/release): | ||
| - `pull-ci-openshift-addon-operator-master-e2e-binary-build-success` - E2E binary build verification | ||
| - `pull-ci-openshift-addon-operator-master-coverage` - Code coverage analysis (with Codecov) | ||
| - `pull-ci-openshift-addon-operator-master-lint` - Linting checks | ||
| - `pull-ci-openshift-addon-operator-master-test` - Unit tests | ||
| - `pull-ci-openshift-addon-operator-master-validate` - Validation checks | ||
|
|
||
| **Tekton Pipelines** (configured in `.tekton/`): | ||
| - `addon-operator-pull-request` - Main PR pipeline (docker build with OCI-TA) | ||
| - `addon-operator-e2e-pull-request` - E2E testing pipeline | ||
| - `addon-operator-pko-pull-request` - PKO (Package Operator) pipeline | ||
| - Corresponding `-push` pipelines for merged commits | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Tekton pipeline files exist and check for Prow job name references
echo "=== Checking Tekton pipeline files ==="
# Verify .tekton/ directory and documented pipeline files
if [ -d ".tekton" ]; then
echo "✓ .tekton/ directory exists"
# Check for specific pipeline files mentioned in lines 237-240
for pipeline in \
"addon-operator-pull-request.yaml" \
"addon-operator-e2e-pull-request.yaml" \
"addon-operator-pko-pull-request.yaml" \
"addon-operator-push.yaml"
do
if [ -f ".tekton/$pipeline" ]; then
echo " ✓ $pipeline exists"
else
echo " ✗ $pipeline not found"
fi
done
else
echo "✗ .tekton/ directory not found"
fi
echo ""
echo "=== Checking for Prow job name references ==="
# Search for references to the documented Prow job names
# Note: These may not exist in this repo if they're defined in openshift/release
job_names=(
"pull-ci-openshift-addon-operator-master-e2e-binary-build-success"
"pull-ci-openshift-addon-operator-master-coverage"
"pull-ci-openshift-addon-operator-master-lint"
"pull-ci-openshift-addon-operator-master-test"
"pull-ci-openshift-addon-operator-master-validate"
)
for job in "${job_names[@]}"; do
if rg -q "$job" .; then
echo " ✓ Found reference to: $job"
else
echo " ℹ No reference found for: $job (may be defined in openshift/release)"
fi
doneRepository: openshift/addon-operator
Length of output: 944
Fix Tekton pipeline name/docs in SKILL.md (E2E file mismatch)
In .claude/skills/prow-ci/SKILL.md (lines 72-83), the listed Tekton file addon-operator-e2e-pull-request.yaml is not present in this repo’s .tekton/ directory, while addon-operator-pull-request.yaml, addon-operator-pko-pull-request.yaml, and addon-operator-push.yaml do exist—update the documentation to the actual E2E pipeline filename/name (or point to where it’s defined). The documented Prow job names have no references in this repo, which matches the “configured in openshift/release” note.
🤖 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 around lines 69 - 83, Update
.claude/skills/prow-ci/SKILL.md to correct the Tekton pipeline name: replace the
non-existent reference to addon-operator-e2e-pull-request with the actual
pipeline filename(s) found in .tekton/ (e.g., addon-operator-pull-request,
addon-operator-pko-pull-request, addon-operator-push) or add a brief note
pointing to where the E2E pipeline is defined; also leave the Prow job list
as-is but keep the “configured in openshift/release” qualifier so readers know
those jobs aren’t in this repo.
| ## Debugging CI Failures | ||
|
|
||
| ### Step 1: Identify Failed Job | ||
| ```bash | ||
| gh pr checks PR_NUMBER | ||
| ``` | ||
|
|
||
| ### Step 2: Access Prow UI | ||
| Open the Prow link from PR checks or construct manually: | ||
| ```text | ||
| https://prow.ci.openshift.org/?repo=openshift%2Faddon-operator&type=presubmit | ||
| ``` | ||
|
|
||
| ### Step 3: Review Logs | ||
| Click on failed job → "Build Log" tab | ||
|
|
||
| ### Step 4: Check Artifacts | ||
| Look for: | ||
| - Test failure logs | ||
| - Coverage reports | ||
| - Generated artifacts | ||
|
|
||
| ### Step 5: Reproduce Locally | ||
| Many Prow jobs can be reproduced with: | ||
| ```bash | ||
| # For unit tests (matches: pull-ci-...-test) | ||
| make go-test | ||
|
|
||
| # For linting (matches: pull-ci-...-lint) | ||
| make go-check | ||
| # OR use pre-commit for comprehensive linting | ||
| pre-commit run --all-files | ||
|
|
||
| # For validation (matches: pull-ci-...-validate) | ||
| make validate | ||
|
|
||
| # For coverage (matches: pull-ci-...-coverage) | ||
| make coverage | ||
|
|
||
| # For E2E binary build (matches: pull-ci-...-e2e-binary-build-success) | ||
| make e2e-binary-build | ||
|
|
||
| # For container builds (Tekton pipelines) | ||
| make docker-build | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify make targets referenced in the prow-ci skill documentation
# List of make targets documented in the skill
targets=(
"go-test"
"go-check"
"validate"
"coverage"
"e2e-binary-build"
"docker-build"
)
echo "=== Checking for Makefile ==="
if [ -f "Makefile" ]; then
echo "✓ Makefile exists"
echo ""
echo "=== Verifying documented make targets ==="
for target in "${targets[@]}"; do
if rg -q "^${target}:" Makefile; then
echo " ✓ Target '$target' exists"
else
echo " ✗ Target '$target' not found in Makefile"
fi
done
else
echo "✗ Makefile not found at repository root"
fi
# Also check for common variations
echo ""
echo "=== Available test-related targets ==="
rg "^[a-zA-Z0-9_-]+:.*test" Makefile || echo "No test-related targets found"Repository: openshift/addon-operator
Length of output: 489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
targets=(
"go-test"
"go-check"
"validate"
"coverage"
"e2e-binary-build"
"docker-build"
)
echo "=== Search for documented make targets across Makefile-related files ==="
# Only scan likely make fragments to avoid huge output
files=$(fd -t f 'Makefile*' -d 6 --hidden . || true)
files2=$(fd -t f '*.mk' -d 6 --hidden . || true)
all_files=$(printf "%s\n%s\n" "$files" "$files2" | sed '/^$/d' | sort -u)
if [ -z "$all_files" ]; then
echo "No Makefile* or *.mk files found within depth 6."
exit 0
fi
for target in "${targets[@]}"; do
echo ""
echo "--- Target: $target ---"
rg -n --no-heading --max-count 20 -S "^[[:space:]]*${target}[[:space:]]*:" $all_files || echo "Not found as a target definition in Makefile*/.mk files."
doneRepository: openshift/addon-operator
Length of output: 908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
targets=(
"go-test"
"go-check"
"validate"
"coverage"
"e2e-binary-build"
"docker-build"
)
echo "=== Locate Makefile-related files ==="
makefiles=$(fd -t f --glob 'Makefile*' -d 10 --hidden . || true)
mkfiles=$(fd -t f --glob '*.mk' -d 10 --hidden . || true)
all_files=$(printf "%s\n%s\n" "$makefiles" "$mkfiles" | sed '/^$/d' | sort -u)
if [ -z "$all_files" ]; then
echo "No Makefile*/.mk files found within depth 10."
exit 0
fi
echo "Found $(printf "%s\n" "$all_files" | wc -l | tr -d ' ') files to scan."
echo "=== Check for target rule definitions (e.g. ^go-check:) ==="
for target in "${targets[@]}"; do
echo ""
echo "--- Target: $target ---"
rg -n --no-heading --max-count 20 -S "^[[:space:]]*${target}[[:space:]]*:" $all_files || echo "Not found as a target rule definition in scanned Makefile*/.mk files."
done
echo ""
echo "=== Check for .PHONY declarations (e.g. .PHONY: go-check) ==="
for target in "${targets[@]}"; do
echo ""
echo "--- PHONY: $target ---"
rg -n --no-heading --max-count 20 -S "(^|[[:space:]])\\.PHONY:.*\\b${target}\\b|\\b${target}\\b" $all_files || echo "No .PHONY mention found for $target in scanned files."
doneRepository: openshift/addon-operator
Length of output: 5610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for likely e2e make target names/mentions across Makefile and mk files
rg -n --hidden -S \
-g'Makefile*' \
-g'*.mk' \
'e2e-binary-build|e2e-binary|binary-build|e2e.*(build|test)|make e2e' . || trueRepository: openshift/addon-operator
Length of output: 50
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden -S \
'e2e-binary-build|e2e-binary|binary-build-success|pull-ci-.*e2e|e2e.*binary' . || trueRepository: openshift/addon-operator
Length of output: 645
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden -S \
-g'Makefile*' \
-g'*.mk' \
'^[[:space:]]*e2e-binary-build[[:space:]]*:' . || trueRepository: openshift/addon-operator
Length of output: 50
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for e2e-binary-build rule definitions (colon) ==="
rg -n --hidden -S '^[[:space:]]*e2e-binary-build[[:space:]]*:' Makefile* **/*.mk **/*.mk 2>/dev/null || true
rg -n --hidden -S 'e2e-binary-build[[:space:]]*:' . || true
echo ""
echo "=== Search for e2e-binary-build in Makefile*/.mk files ==="
rg -n --hidden -S -g'Makefile*' -g'*.mk' 'e2e-binary-build' . || true
echo ""
echo "=== Search for .PHONY mentioning e2e-binary-build ==="
rg -n --hidden -S '\\.PHONY.*\\be2e-binary-build\\b|\\be2e-binary-build\\b.*\\.PHONY' . || trueRepository: openshift/addon-operator
Length of output: 244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== All occurrences of e2e-binary-build in repo ==="
rg -n --hidden -S 'e2e-binary-build' . || true
echo ""
echo "=== All occurrences of e2e-binary-build-success in repo ==="
rg -n --hidden -S 'e2e-binary-build-success' . || trueRepository: openshift/addon-operator
Length of output: 1095
Fix E2E make target reference in prow-ci skill
go-check, validate, coverage, and docker-build are defined (via ./boilerplate/openshift/golang-osd-operator/standard.mk), and go-test also exists in the root Makefile. The guide’s make e2e-binary-build instruction is the only one that doesn’t correspond to any e2e-binary-build: rule (no occurrences of e2e-binary-build as a make target in Makefile/*.mk within the repo), so following it will fail with “No rule to make target…”. Update the target name in ./.claude/skills/prow-ci/SKILL.md or add the missing make target.
🤖 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 around lines 84 - 128, The SKILL.md entry
references a non-existent make target "e2e-binary-build"; update the document to
either use the actual E2E binary build make target that exists in the repo
(replace "make e2e-binary-build" with the correct target name) or add a
corresponding make rule named "e2e-binary-build" to the repo; look for the other
existing targets referenced in the file ("go-test", "go-check", "validate",
"coverage", "docker-build") to match style and naming when choosing the correct
replacement or when adding the new rule.
| 6. Test the skill workflow | ||
|
|
||
| **Directory structure**: | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The directory structure code block should specify a language for proper rendering and linting compliance.
📝 Proposed fix
-```
+```text
.claude/skills/
├── README.md
└── skillname/🧰 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 at line 66, Update the fenced code block in
.claude/skills/README.md that shows the directory structure to include a
language specifier (e.g., change the opening "```" to "```text") so the block
for the directory tree renders and lints correctly; locate the directory
structure snippet in README.md and modify its opening fence accordingly.
| paths = [ | ||
| '''test/fixtures/.*''', | ||
| '''test/deploy/.*''', | ||
| '''.*_test\.go''', | ||
| '''boilerplate/.*''', | ||
| '''vendor/.*''', | ||
| '''zz_generated\..*\.go''', | ||
| ] |
There was a problem hiding this comment.
❓ Verification inconclusive
🏁 Script executed:
#!/usr/bin/env bash
# Find current broad gitleaks allowlist and candidate test fixture paths
rg -n -C2 "_test\\\\\\.go|test/fixtures|test/deploy" .gitleaks.toml
fd -a "_test.go$" testRepository: openshift/addon-operator
Length of output: 340
Narrow the gitleaks path exception for Go tests (.*_test\.go)
.gitleaks.toml’s paths = [...] block includes .*_test\.go, which matches every Go *_test.go file; this is overly broad and can hide leaked secrets in tests—prefer limiting exceptions to specific fixture/deploy test paths instead.
Suggested tightening
paths = [
'''test/fixtures/.*''',
'''test/deploy/.*''',
- '''.*_test\.go''',
'''boilerplate/.*''',
'''vendor/.*''',
'''zz_generated\..*\.go''',
]📝 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 gitleaks config is too permissive
because the paths array contains the pattern '.*_test\.go' which excludes all Go
test files; change the 'paths' entry by removing or narrowing that pattern and
instead whitelist only explicit test directories or file-glob anchors (e.g.,
limit to specific directories like test/fixtures or test/deploy or use anchored
regexes that target only those paths) so that only intended fixture/deploy test
files are exempted; update the 'paths' array entry that currently contains
'.*_test\.go' accordingly.
| exit 1 | ||
| fi | ||
|
|
||
| prek run --config hack/prek.ci.toml --all-files |
There was a problem hiding this comment.
Use a repo-root-resolved config path to avoid cwd-dependent failures.
hack/prek.ci.toml is resolved relative to the current working directory, not the script location. Invoking this script outside repo root can fail unexpectedly.
Suggested patch
#!/usr/bin/env bash
set -euo pipefail
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
+REPO_ROOT="$(cd -- "${SCRIPT_DIR}/.." && pwd)"
+
if ! command -v prek &>/dev/null; then
echo "Error: prek is not installed. Install with: uv tool install prek" >&2
exit 1
fi
-prek run --config hack/prek.ci.toml --all-files
+prek run --config "${REPO_ROOT}/hack/prek.ci.toml" --all-files📝 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.
| prek run --config hack/prek.ci.toml --all-files | |
| #!/usr/bin/env bash | |
| set -euo pipefail | |
| SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" | |
| REPO_ROOT="$(cd -- "${SCRIPT_DIR}/.." && pwd)" | |
| if ! command -v prek &>/dev/null; then | |
| echo "Error: prek is not installed. Install with: uv tool install prek" >&2 | |
| exit 1 | |
| fi | |
| prek run --config "${REPO_ROOT}/hack/prek.ci.toml" --all-files |
🤖 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/ci.sh` at line 9, The command "prek run --config hack/prek.ci.toml
--all-files" relies on the current working directory; change it to compute the
repository root from the script location and pass an absolute config path
instead. In the hack/ci.sh script, determine REPO_ROOT (e.g., from the script
dirname/parent) and replace the literal "hack/prek.ci.toml" in the "prek run
--config hack/prek.ci.toml --all-files" invocation with
"$REPO_ROOT/hack/prek.ci.toml" (or equivalent absolute path variable) so the
config is resolved independent of cwd.
| ## Pre-commit Integration | ||
|
|
||
| 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.
Clarify the pre-commit integration status.
The YAML example on lines 267-271 suggests go-test is configured in pre-commit, but line 273 states "This is NOT in current pre-commit config." This contradiction will confuse readers.
Consider restructuring this section to clarify upfront:
## Pre-commit Integration
Tests are **not** automatically run in pre-commit hooks (too slow for quick commits).
Instead, run manually before pushing:
```bash
make go-testWhy not in pre-commit? The full test suite can take 2+ minutes, which would slow down the commit workflow. Pre-commit hooks are reserved for fast checks (<30s).
If you want to enable it locally (optional), add this to .pre-commit-config.yaml:
- id: go-test
entry: make go-test
files: '\.go$'
<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 @TESTING.md around lines 264 - 274, Update the "Pre-commit Integration"
section in TESTING.md to remove the contradictory example and make the status
explicit: start with a sentence that tests are NOT run by pre-commit hooks (too
slow), then instruct readers to run make go-test manually before pushing, add
a brief rationale that the full suite takes 2+ minutes and pre-commit is
reserved for fast checks, and optionally note how to enable it locally by adding
the shown go-test hook snippet to .pre-commit-config.yaml; edit the "Pre-commit
Integration" header and body so the YAML example is presented only as an
optional local enablement and the main guidance clearly states tests are not run
automatically.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #750 +/- ##
=======================================
Coverage 59.24% 59.24%
=======================================
Files 62 62
Lines 4125 4125
=======================================
Hits 2444 2444
Misses 1532 1532
Partials 149 149 🚀 New features to boost your workflow:
|
devppratik
force-pushed
the
add-claude-and-docs
branch
3 times, most recently
from
a9062f4 to
8a28197
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
8a28197 to
6202233
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