docs, feat(SREP-4460, SREP-4926: Add Standardized Claude hooks, skill, agents. Update standardised docs)#422
Conversation
|
[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 27 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)
WalkthroughAdds CLAUDE agent specifications, repository safety hooks and scripts, prek/pre-commit CI configs, gitleaks rules, skills docs, CI entrypoint, and comprehensive contributor/developer/testing documentation. ChangesClaude Agents & Developer Workflow
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@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. |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (1)
hack/ci.sh (1)
9-9: ⚡ Quick winMake CI config path independent of current working directory.
Using a CWD-relative
--configcan break when this script is called from outside repo root. Resolve the repo root from the script path and pass an absolute config path.Proposed fix
#!/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🤖 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 CI invocation currently uses a CWD-relative config ("--config hack/prek.ci.toml"); make it path-independent by resolving the repo root from the script location and passing an absolute config path to prek. Update the invocation that runs "prek run" to first compute the repo root (e.g., from "${BASH_SOURCE[0]}" or "$0") and join it with "hack/prek.ci.toml", then call "prek run --config <absolute-path-to-hack/prek.ci.toml> --all-files" so the config is found regardless of the current working directory.
🤖 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:
- Line 10: Replace the incorrect project name "OCM Agent Operator" with the
correct "aws-vpce-operator" throughout the .claude/agents/ci-agent.md file
(including the occurrence noted around line 177); update any headings, metadata,
and in-document references so the file consistently identifies the project as
aws-vpce-operator and verify there are no remaining instances of the old name.
- Around line 24-30: Update the Tekton pipeline filenames listed (e.g., replace
occurrences of `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`, and
`ocm-agent-operator-pko-pull-request.yaml`) to the correct repo-specific names
using the `aws-vpce-operator-*` prefix so the docs point to existing manifests
(e.g., `aws-vpce-operator-pull-request.yaml`, `aws-vpce-operator-push.yaml`,
`aws-vpce-operator-e2e-pull-request.yaml`, `aws-vpce-operator-e2e-push.yaml`,
`aws-vpce-operator-pko-push.yaml`, `aws-vpce-operator-pko-pull-request.yaml`),
and make the same replacements for the other referenced occurrences (lines noted
in the comment).
- Around line 31-39: The CI detection currently greps .tekton/*.yaml for literal
strings like "golangci-lint", "gitleaks", "go test", and "go build", which
breaks when Tekton tasks are referenced via pipelineRef; update the detection
logic to resolve pipelineRef entries instead of raw string grep: when a manifest
contains a pipelineRef, read its name and search the repo for the corresponding
pipeline definition (e.g., pipelines/<pipeline-name>/pipeline.yaml or any YAML
with metadata.name == <pipelineRef.name>), then inspect that pipeline's
taskRefs/steps for golangci-lint/gitleaks/go test/go build; ensure fallback
behavior still scans inline pipeline specs in .tekton/*.yaml and treat
unresolved pipelineRefs as unknown rather than false positives.
- Around line 55-60: The parity check commands that run grep "rev:"
.pre-commit-config.yaml | grep golangci-lint and the gitleaks variant fail
because the repo name is not on the same line as the rev field; update the check
to first find the hook block for "golangci-lint" (and "gitleaks") and then
extract the subsequent rev value (e.g., locate the line containing the hook
repo/name and read the following indented rev: line or use a YAML-aware lookup),
replacing the current two-stage grep pipelines in .claude/agents/ci-agent.md so
the command reliably prints the hook's rev for both golangci-lint and gitleaks.
In @.claude/agents/docs-agent.md:
- Line 10: Update the intro text that currently says "OCM Agent Operator" to use
the correct repository name `aws-vpce-operator`; locate the string "OCM Agent
Operator" in docs-agent.md (intro section) and replace it with
`aws-vpce-operator`, and scan the file for any other occurrences of "OCM Agent
Operator" or similar variants to keep naming consistent across the document.
- Around line 188-198: The fenced example block in .claude/agents/docs-agent.md
is missing a language tag causing MD040; update the opening fence from ``` to
```text (or another appropriate tag like ```md) for the block that contains
"Updated: DEVELOPMENT.md ... Validated: ..." so the markdown linter recognizes
the code block and leave the closing ``` unchanged.
In @.claude/agents/lint-agent.md:
- Line 10: Update the agent brief text that currently says "OCM Agent Operator"
to use the correct repository/operator name "aws-vpce-operator"; locate the
phrase "OCM Agent Operator" in the agent brief content (the agent
description/title) and replace it with "aws-vpce-operator" so the prompt/context
matches this repo and avoids drift.
In @.claude/agents/security-agent.md:
- Line 10: Update the introductory sentence that currently reads "OCM Agent
Operator" to the correct repository/operator name used in this PR; locate the
phrase "OCM Agent Operator" in the Security Agent intro (around the opening
paragraph in .claude/agents/security-agent.md) and replace it with the actual
repo/operator name used by this PR so the guidance matches the codebase.
In @.claude/agents/test-agent.md:
- Line 10: Replace the incorrect repository name "OCM Agent Operator" in the
Test Agent intro with the current repository/operator name "aws-vpce-operator";
update the string occurrence in the test-agent.md intro so the sentence reads
with "aws-vpce-operator" for consistency across docs.
In @.claude/hooks/README.md:
- Around line 182-183: Update the README description for the stop hook so it
matches the actual hook script: change the text that currently states "prek run
--all-files" to "prek run --config hack/prek.ci.toml" (referencing the stop hook
command in the hooks README and the stop hook script invocation) so
documentation and implementation are consistent.
- Line 3: The README currently mentions "OCM Agent Operator" which is
inconsistent with this repository's name; update the phrasing in the README (the
string "OCM Agent Operator" on the mentioned line) to the correct repository
name "aws-vpce-operator" so contributor docs consistently reference this
project; ensure any other occurrences of "OCM Agent Operator" in the README are
also replaced to avoid copy/paste confusion.
In @.claude/skills/README.md:
- Around line 66-72: The fenced code block in .claude/skills/ README.md (the
directory tree example) lacks a language identifier which triggers markdownlint
MD040; update the opening triple-backtick to include a language tag (e.g.,
```text) for the tree diagram so the block becomes fenced with a language
identifier and satisfies linting, keeping the block content unchanged.
In @.gitleaks.toml:
- Around line 25-32: The allowlist in the paths array currently contains a
global pattern '''.*_test\.go''' which excludes all Go test files from gitleaks;
remove that entry and limit exemptions to specific fixture patterns (e.g., keep
'''test/fixtures/.*''' and any other explicit fixture/deploy patterns you
intentionally want to allow) so only known fixture paths are excluded; update
the paths array to drop the '''.*_test\.go''' symbol and ensure remaining
entries (like '''test/fixtures/.*''', '''test/deploy/.*''') are the only
test-related exemptions.
In `@TESTING.md`:
- Around line 266-275: Remove the contradictory guidance by keeping the YAML
pre-commit snippet as an example/future option and retaining the authoritative
instruction to run tests manually with `make go-test`; specifically, modify the
section containing the `- id: go-test` snippet and the surrounding sentences so
the YAML block is clearly labeled "example (optional / future)" or "not enabled
in current config", and ensure the lone authoritative statement left is "Run
manually before pushing: `make go-test`" so contributors see only one source of
truth.
---
Nitpick comments:
In `@hack/ci.sh`:
- Line 9: The CI invocation currently uses a CWD-relative config ("--config
hack/prek.ci.toml"); make it path-independent by resolving the repo root from
the script location and passing an absolute config path to prek. Update the
invocation that runs "prek run" to first compute the repo root (e.g., from
"${BASH_SOURCE[0]}" or "$0") and join it with "hack/prek.ci.toml", then call
"prek run --config <absolute-path-to-hack/prek.ci.toml> --all-files" so the
config is found regardless of the current working directory.
🪄 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: a04a178b-9caa-4d59-b033-cd34acd01279
📒 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
| - `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.
Repo-specific Tekton filenames are incorrect for this project.
These entries reference ocm-agent-operator-* pipelines, but this repository’s Tekton files are aws-vpce-operator-* (including the example output pipeline name). This will send contributors to non-existent manifests.
Suggested doc fix
-- `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
+- `aws-vpce-operator-pull-request.yaml`: PR validation
+- `aws-vpce-operator-push.yaml`: Main branch builds
+- `aws-vpce-operator-e2e-pull-request.yaml`: E2E tests on PR
+- `aws-vpce-operator-e2e-push.yaml`: E2E tests on merge
+- `aws-vpce-operator-pko-push.yaml`: PKO deployment
+- `aws-vpce-operator-pko-pull-request.yaml`: PKO validation
...
-Pipeline: ocm-agent-operator-pull-request
+Pipeline: aws-vpce-operator-pull-requestAlso applies to: 246-247
🤖 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 24 - 30, Update the Tekton pipeline
filenames listed (e.g., replace occurrences of
`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`, and
`ocm-agent-operator-pko-pull-request.yaml`) to the correct repo-specific names
using the `aws-vpce-operator-*` prefix so the docs point to existing manifests
(e.g., `aws-vpce-operator-pull-request.yaml`, `aws-vpce-operator-push.yaml`,
`aws-vpce-operator-e2e-pull-request.yaml`, `aws-vpce-operator-e2e-push.yaml`,
`aws-vpce-operator-pko-push.yaml`, `aws-vpce-operator-pko-pull-request.yaml`),
and make the same replacements for the other referenced occurrences (lines noted
in the comment).
| grep "rev:" .pre-commit-config.yaml | grep golangci-lint | ||
| # Should match version in boilerplate pipeline | ||
|
|
||
| # Check gitleaks version | ||
| grep "rev:" .pre-commit-config.yaml | grep gitleaks | ||
| ``` |
There was a problem hiding this comment.
Parity check command currently can’t return the intended hook versions.
grep "rev:" ... | grep golangci-lint (and gitleaks) will usually return nothing because the rev: line does not include the hook repo name on the same line.
Suggested command fix
-# Check pre-commit uses same golangci-lint version as CI
-grep "rev:" .pre-commit-config.yaml | grep golangci-lint
+awk '
+ /repo: .*golangci\/golangci-lint/ {found=1; next}
+ found && /rev:/ {print "golangci-lint " $2; found=0}
+' .pre-commit-config.yaml
-# Check gitleaks version
-grep "rev:" .pre-commit-config.yaml | grep gitleaks
+awk '
+ /repo: .*gitleaks\/gitleaks/ {found=1; next}
+ found && /rev:/ {print "gitleaks " $2; found=0}
+' .pre-commit-config.yaml🤖 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 55 - 60, The parity check commands
that run grep "rev:" .pre-commit-config.yaml | grep golangci-lint and the
gitleaks variant fail because the repo name is not on the same line as the rev
field; update the check to first find the hook block for "golangci-lint" (and
"gitleaks") and then extract the subsequent rev value (e.g., locate the line
containing the hook repo/name and read the following indented rev: line or use a
YAML-aware lookup), replacing the current two-stage grep pipelines in
.claude/agents/ci-agent.md so the command reliably prints the hook's rev for
both golangci-lint and gitleaks.
| ``` | ||
| .claude/skills/ | ||
| ├── README.md | ||
| └── skillname/ | ||
| ├── SKILL.md # Required: skill definition | ||
| └── reference/ # Optional: supporting docs | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block to avoid markdownlint failures.
Line 66 uses a plain triple-backtick fence; markdownlint MD040 expects a language tag.
Suggested fix
-```
+```text
.claude/skills/
├── README.md
└── skillname/
├── SKILL.md # Required: skill definition
└── reference/ # Optional: supporting docs</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/README.md around lines 66 - 72, The fenced code block in
.claude/skills/ README.md (the directory tree example) lacks a language
identifier which triggers markdownlint MD040; update the opening triple-backtick
to include a language tag (e.g., ```text) for the tree diagram so the block
becomes fenced with a language identifier and satisfies linting, keeping the
block content unchanged.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| paths = [ | ||
| '''test/fixtures/.*''', | ||
| '''test/deploy/.*''', | ||
| '''.*_test\.go''', | ||
| '''boilerplate/.*''', | ||
| '''vendor/.*''', | ||
| '''zz_generated\..*\.go''', | ||
| ] |
There was a problem hiding this comment.
Narrow the test-file allowlist to fixtures only.
Line 28 (.*_test\.go) globally excludes all Go test files from gitleaks scanning, which creates a meaningful blind spot for credential leaks. Keep exemptions scoped to known fixture paths/patterns instead of all _test.go files. 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.
Suggested change
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 allowlist in the paths array
currently contains a global pattern '''.*_test\.go''' which excludes all Go test
files from gitleaks; remove that entry and limit exemptions to specific fixture
patterns (e.g., keep '''test/fixtures/.*''' and any other explicit
fixture/deploy patterns you intentionally want to allow) so only known fixture
paths are excluded; update the paths array to drop the '''.*_test\.go''' symbol
and ensure remaining entries (like '''test/fixtures/.*''', '''test/deploy/.*''')
are the only test-related exemptions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #422 +/- ##
=======================================
Coverage 40.69% 40.69%
=======================================
Files 32 32
Lines 2150 2150
=======================================
Hits 875 875
Misses 1171 1171
Partials 104 104 🚀 New features to boost your workflow:
|
17b785f to
917afb6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.claude/agents/docs-agent.md (1)
188-198:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced output block.
This block still lacks a fence language and can fail markdown lint checks (MD040).
Proposed 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 @@ - All command examples tested - Links checked</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.claude/agents/docs-agent.md around lines 188 - 198, The fenced output block
in .claude/agents/docs-agent.md is missing a language tag which triggers MD040;
update the opening fence fromtotext (or another appropriate tag likewith a language identifier, keeping the content unchanged and leaving the closing ``` as-is..claude/hooks/README.md (2)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRepository name is inconsistent in docs.
Line 3 still says “OCM Agent Operator”; this should be
aws-vpce-operatorto avoid copy/paste confusion in contributor-facing docs.🤖 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 at line 3, Update the repository name in the hooks README by replacing the incorrect string "OCM Agent Operator" with the correct repository name "aws-vpce-operator" in the README.md content (look for the sentence beginning "Security and validation hooks for ...") so contributor-facing docs reflect the correct project and avoid copy/paste confusion.
182-183:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStop-hook command description conflicts with the rest of this README.
Line 182 says the stop hook runs
prek run --all-files, but Line 72 documentsprek run --config hack/prek.ci.toml. Please make these consistent with actual hook behavior.🤖 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 182 - 183, README contains conflicting descriptions for the stop hook: one place says the stop hook runs "prek run --all-files" while another documents "prek run --config hack/prek.ci.toml"; update the stop-hook description to match the actual hook implementation by replacing the incorrect command with the real one (reference the "stop hook" entry and the exact command string "prek run --all-files" or "prek run --config hack/prek.ci.toml" as appropriate) so both the stop hook section and the earlier command listing use the same command string and wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/agents/README.md:
- Line 3: Update the intro sentence in the README titled line "Specialized
agents for this operator development workflows." to read "Specialized agents for
this operator’s development workflows." so the grammar and possessive are
corrected; locate the sentence in .claude/agents/README.md and replace the
existing phrase with the revised wording.
In @.claude/hooks/cleanup.sh:
- Around line 3-4: Replace the incorrect repository name in the header comment
at the top of .claude/hooks/cleanup.sh: change the phrase "OCM Agent Operator"
to "aws-vpce-operator" so the file header accurately reflects this repository.
---
Duplicate comments:
In @.claude/agents/docs-agent.md:
- Around line 188-198: The fenced output block in .claude/agents/docs-agent.md
is missing a language tag which triggers MD040; update the opening fence from
``` to ```text (or another appropriate tag like ```diff if you prefer diff
semantics) so the block becomes a fenced code block with a language identifier,
keeping the content unchanged and leaving the closing ``` as-is.
In @.claude/hooks/README.md:
- Line 3: Update the repository name in the hooks README by replacing the
incorrect string "OCM Agent Operator" with the correct repository name
"aws-vpce-operator" in the README.md content (look for the sentence beginning
"Security and validation hooks for ...") so contributor-facing docs reflect the
correct project and avoid copy/paste confusion.
- Around line 182-183: README contains conflicting descriptions for the stop
hook: one place says the stop hook runs "prek run --all-files" while another
documents "prek run --config hack/prek.ci.toml"; update the stop-hook
description to match the actual hook implementation by replacing the incorrect
command with the real one (reference the "stop hook" entry and the exact command
string "prek run --all-files" or "prek run --config hack/prek.ci.toml" as
appropriate) so both the stop hook section and the earlier command listing use
the same command string and wording.
🪄 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: 3f91ded8-0e69-486e-ace5-5c89ee898c50
📒 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 (9)
- .prek-version
- hack/ci.sh
- .claude/agents/security-agent.md
- DEVELOPMENT.md
- .claude/agents/test-agent.md
- hack/prek.ci.toml
- prek.toml
- .claude/agents/lint-agent.md
- CONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (6)
- TESTING.md
- .claude/agents/ci-agent.md
- .claude/hooks/stop-prek-validation.sh
- .claude/settings.json
- .gitleaks.toml
- .claude/hooks/pre-edit.sh
| @@ -0,0 +1,244 @@ | |||
| # Claude Agents | |||
|
|
|||
| Specialized agents for this operator development workflows. | |||
There was a problem hiding this comment.
Fix grammar in the intro sentence.
Current wording is awkward (“this operator development workflows”). Please update to “this operator’s development workflows” for clarity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/agents/README.md at line 3, Update the intro sentence in the README
titled line "Specialized agents for this operator development workflows." to
read "Specialized agents for this operator’s development workflows." so the
grammar and possessive are corrected; locate the sentence in
.claude/agents/README.md and replace the existing phrase with the revised
wording.
| # Cleanup Hook for OCM Agent Operator | ||
| # Runs when Claude Code session stops |
There was a problem hiding this comment.
Header comment uses the wrong repository name.
Line 3 references “OCM Agent Operator”; update it to aws-vpce-operator for consistency with this repo.
🤖 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/cleanup.sh around lines 3 - 4, Replace the incorrect
repository name in the header comment at the top of .claude/hooks/cleanup.sh:
change the phrase "OCM Agent Operator" to "aws-vpce-operator" so the file header
accurately reflects this repository.
917afb6 to
59cb98e
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
59cb98e to
2e629e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.claude/agents/docs-agent.md (1)
44-45: ⚡ Quick winBroaden markdown validation commands beyond repo-root
*.md.Current examples only scan top-level markdown files, so nested docs can be skipped during validation.
Based on learnings: Run pre-commit run --all-files before submitting a PR.Proposed command updates
- grep '```bash' -A 10 *.md | grep '^make\|^go\|^ginkgo' + find . -name "*.md" -not -path "./vendor/*" -not -path "./.git/*" -print0 \ + | xargs -0 grep -nE '```bash|^(make|go|ginkgo)\b' - grep -E '```$' *.md # Code blocks without language - grep -E '\[.*\]\(\./' *.md # Relative links to check + find . -name "*.md" -not -path "./vendor/*" -not -path "./.git/*" -print0 \ + | xargs -0 grep -nE '```$' # Code blocks without language + find . -name "*.md" -not -path "./vendor/*" -not -path "./.git/*" -print0 \ + | xargs -0 grep -nE '\[.*\]\(\./' # Relative links to check - grep '```bash' *.md | grep 'make ' | sed 's/.*make \([a-z-]*\).*/\1/' | sort -u + find . -name "*.md" -not -path "./vendor/*" -not -path "./.git/*" -print0 \ + | xargs -0 grep -hE '```bash|make ' \ + | sed -n 's/.*make \([a-zA-Z0-9._-]*\).*/\1/p' \ + | sort -uAlso applies to: 59-60, 176-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, Replace the repo-root-only markdown grep invocations (e.g., the commands matching "grep '```bash' -A 10 *.md", "grep -E '```$' *.md", "grep -E '\\[.*\\]\\(\\.\\/ ' *.md", and "grep '```bash' *.md | grep 'make '") with recursive find+xargs pipelines that search all *.md under the tree while excluding ./vendor and .git (so nested docs aren't skipped); update the three affected blocks (the grep '```bash' block, the code-block/relative-link checks, and the make-target extraction pipeline) accordingly and apply the same changes at the other occurrences noted (lines around 59-60 and 176-183), and ensure the docs recommend running "pre-commit run --all-files" before submitting a PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/prow-ci/SKILL.md:
- Around line 113-116: Replace the explicit pre-commit invocation with the
repo-standard wrapper command: change the line containing "pre-commit run
--all-files" to use "prek" so the validation flow is consistent with this PR;
update the SKILL.md section that currently shows the commands (the lines with
"make go-check" and "pre-commit run --all-files") to instead show "make
go-check" followed by "prek".
---
Nitpick comments:
In @.claude/agents/docs-agent.md:
- Around line 44-45: Replace the repo-root-only markdown grep invocations (e.g.,
the commands matching "grep '```bash' -A 10 *.md", "grep -E '```$' *.md", "grep
-E '\\[.*\\]\\(\\.\\/ ' *.md", and "grep '```bash' *.md | grep 'make '") with
recursive find+xargs pipelines that search all *.md under the tree while
excluding ./vendor and .git (so nested docs aren't skipped); update the three
affected blocks (the grep '```bash' block, the code-block/relative-link checks,
and the make-target extraction pipeline) accordingly and apply the same changes
at the other occurrences noted (lines around 59-60 and 176-183), and ensure the
docs recommend running "pre-commit run --all-files" before submitting a PR.
🪄 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: 1b1c2e67-4748-4e3e-8ea8-0b0ee579a1a0
📒 Files selected for processing (21)
.claude/agents/README.md.claude/agents/ci-agent.md.claude/agents/docs-agent.md.claude/agents/lint-agent.md.claude/agents/security-agent.md.claude/agents/test-agent.md.claude/hooks/README.md.claude/hooks/cleanup.sh.claude/hooks/pre-edit.sh.claude/hooks/stop-prek-validation.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.gitleaks.toml.prek-versionCONTRIBUTING.mdDEVELOPMENT.mdTESTING.mdhack/ci.shhack/prek.ci.tomlprek.toml
✅ Files skipped from review due to trivial changes (8)
- .claude/hooks/cleanup.sh
- .prek-version
- DEVELOPMENT.md
- .claude/agents/test-agent.md
- .claude/agents/lint-agent.md
- CONTRIBUTING.md
- TESTING.md
- .claude/agents/ci-agent.md
🚧 Files skipped from review as they are similar to previous changes (7)
- hack/ci.sh
- .claude/settings.json
- .claude/agents/security-agent.md
- .claude/hooks/pre-edit.sh
- hack/prek.ci.toml
- prek.toml
- .gitleaks.toml
| make go-check | ||
| # OR use pre-commit for comprehensive linting | ||
| pre-commit run --all-files | ||
|
|
There was a problem hiding this comment.
Use prek command here for consistency with repo-standard validation flow.
This section currently points to pre-commit run --all-files, while this PR standardizes contributor flow around prek.
Suggested doc fix
-# OR use pre-commit for comprehensive linting
-pre-commit run --all-files
+# OR use prek for comprehensive linting
+prek run --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.
| make go-check | |
| # OR use pre-commit for comprehensive linting | |
| pre-commit run --all-files | |
| make go-check | |
| # OR use prek for comprehensive linting | |
| prek run --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 @.claude/skills/prow-ci/SKILL.md around lines 113 - 116, Replace the explicit
pre-commit invocation with the repo-standard wrapper command: change the line
containing "pre-commit run --all-files" to use "prek" so the validation flow is
consistent with this PR; update the SKILL.md section that currently shows the
commands (the lines with "make go-check" and "pre-commit run --all-files") to
instead show "make go-check" followed by "prek".
|
@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. |
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
New Features
Documentation
Chores