OSAC-2402: update AGENTS.md/CLAUDE.md for Helm and deprecated Kustomize mode - #396
Conversation
|
@amej: This pull request references OSAC-1962 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change updates repository and AI-tooling documentation for Helm validation, deployment commands, protected submodule boundaries, shell conventions, and Claude Code workflows. ChangesRepository and AI workflow guidance
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeRabbit help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
|
@coderabbitai rate limit |
|
Your plan includes PR reviews subject to rate limits. Reviews are available now. |
|
@coderabbitai run pre-merge checks |
✅ Actions performedRunning Pre-merge checks. Results will be posted in the summary comment once complete. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@coderabbitai run pre-merge checks |
✅ Actions performedRunning Pre-merge checks. Results will be posted in the summary comment once complete. |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 23-27: Update the submodule initialization instructions in
AGENTS.md to remove the --remote flag from the normal git submodule update
command, preserving pinned snapshots during builds and deploys. Keep remote
updates limited to the explicit make sync-charts synchronization workflow.
- Around line 36-42: Update all documented bare make helm-template and make
helm-validate commands, including the referenced sections, to explicitly set
VALUES_FILE=values/development/values.yaml before the make command. Keep the
deployment example consistent with this valid path and remove any instructions
that invoke the invalid default.
- Around line 277-290: Update the validation instructions in AGENTS.md,
including the repeated bugfix workflow, to document the complete required
sequence: strict YAML lint, pre-commit checks, Helm lint, Helm validation, all
Kustomize builds, and image-tag synchronization. Add the repository’s
established commands for running every Kustomize build and synchronizing image
tags immediately after Helm validation.
In `@CLAUDE.md`:
- Line 36: Update the submodule protection guidance in CLAUDE.md to include
base/bare-metal-fulfillment-operator/ and base/osac-ui/ alongside the existing
protected paths, ensuring Write is prohibited for every submodule directory.
- Around line 62-63: The commit example in CLAUDE.md only includes the DCO
signoff and omits the required Assisted-by trailer. Update the git commit
example to include “Assisted-by: Claude Code <noreply@anthropic.com>” while
retaining the -s signoff option.
- Around line 42-57: Update the validation and Kustomize workflow examples in
the documentation to use absolute paths consistently, including replacing “.”,
“overlays/*/”, and “scripts/...” references. Define or reuse a repository-root
variable and construct all paths from it, including the commands covered by the
related occurrence.
- Around line 13-17: Remove stale Kustomize references from the affected
sections of CLAUDE.md, including overlays/base kustomization files and Kustomize
build instructions. Align the guidance with AGENTS.md: overlays contain only
secret files/ directories, legacy guidance should mention only existing files,
and build loops must not require secret-only overlays to contain or build
kustomization.yaml manifests.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2c3cafc3-35ba-4b74-be9a-009ce8e6209b
📒 Files selected for processing (5)
.ai-bot/feedback-workflow.md.ai-bot/instructions.md.ai-bot/new-ticket-workflow.mdAGENTS.mdCLAUDE.md
💤 Files with no reviewable changes (3)
- .ai-bot/new-ticket-workflow.md
- .ai-bot/instructions.md
- .ai-bot/feedback-workflow.md
- AGENTS.md: Remove --remote from normal submodule update (preserve pinned snapshots) - AGENTS.md: Fix all VALUES_FILE defaults to use explicit values/development/values.yaml - AGENTS.md: Add documentation for generate-chart-versions.sh and get-chart-version.sh - CLAUDE.md: Update to Helm-based description, remove stale Kustomize references - CLAUDE.md: Add all submodule directories to Write protection - CLAUDE.md: Update examples to use absolute paths - CLAUDE.md: Add VALUES_FILE override and Assisted-by trailer to examples Addresses 6 of 7 CodeRabbit inline comments. Skipped comment about adding Kustomize validation to suite as it was deprecated in OSAC-1677. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Ameya Sathe <asathe@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@AGENTS.md`:
- Around line 72-95: Add the text language specifier to both fenced tree-listing
blocks in the Kustomize and Helm architecture documentation, changing each
opening fence to ```text while leaving the listed content unchanged.
In `@CLAUDE.md`:
- Around line 20-21: Update the shell-script guidance in CLAUDE.md to use a
valid command enabling all three options, replacing the ambiguous `set -o
nounset errexit pipefail` form with `set -euo pipefail` or separate `-o` flags
for nounset, errexit, and pipefail.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: cd835a4b-e69b-48ff-bbed-52117df126d8
📒 Files selected for processing (5)
.ai-bot/feedback-workflow.md.ai-bot/instructions.md.ai-bot/new-ticket-workflow.mdAGENTS.mdCLAUDE.md
💤 Files with no reviewable changes (3)
- .ai-bot/new-ticket-workflow.md
- .ai-bot/feedback-workflow.md
- .ai-bot/instructions.md
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.ai-bot/feedback-workflow.md:
- Around line 25-29: Update the shell-command code fence in the documented
workflow to use the bash language label (` ```bash `) and add blank lines
immediately before and after the fenced block to satisfy markdownlint.
- Line 1: Add a top-level H1 heading at the beginning of the feedback workflow
document before the existing prose, using a concise title that describes the
document and satisfies markdownlint MD041.
In @.ai-bot/instructions.md:
- Line 1: Update the markdown in instructions.md to satisfy markdownlint: add a
top-level heading, label the first two fenced code blocks with bash, and insert
blank lines immediately before and after every fenced block, including the
sections referenced by the review.
In @.ai-bot/new-ticket-workflow.md:
- Line 1: Add a top-level heading to the workflow document, add blank lines
before and after the command block, and label its fenced code block as bash to
resolve the markdownlint violations.
In `@AGENTS.md`:
- Around line 26-30: Update the “Submodules (READ ONLY)” guidance in AGENTS.md
to explicitly require all Git commands to be executed from the installer root,
never from within submodule directories; clarify that commands such as git -C
base/... are also prohibited.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6bc8d25f-5a51-4f5e-bac8-2f5067e620c2
📒 Files selected for processing (5)
.ai-bot/feedback-workflow.md.ai-bot/instructions.md.ai-bot/new-ticket-workflow.mdAGENTS.mdCLAUDE.md
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🤖 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 @.ai-bot/feedback-workflow.md:
- Around line 25-29: Update the shell-command code fence in the documented
workflow to use the bash language label (` ```bash `) and add blank lines
immediately before and after the fenced block to satisfy markdownlint.
- Line 1: Add a top-level H1 heading at the beginning of the feedback workflow
document before the existing prose, using a concise title that describes the
document and satisfies markdownlint MD041.
In @.ai-bot/instructions.md:
- Line 1: Update the markdown in instructions.md to satisfy markdownlint: add a
top-level heading, label the first two fenced code blocks with bash, and insert
blank lines immediately before and after every fenced block, including the
sections referenced by the review.
In @.ai-bot/new-ticket-workflow.md:
- Line 1: Add a top-level heading to the workflow document, add blank lines
before and after the command block, and label its fenced code block as bash to
resolve the markdownlint violations.
In `@AGENTS.md`:
- Around line 26-30: Update the “Submodules (READ ONLY)” guidance in AGENTS.md
to explicitly require all Git commands to be executed from the installer root,
never from within submodule directories; clarify that commands such as git -C
base/... are also prohibited.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6bc8d25f-5a51-4f5e-bac8-2f5067e620c2
📒 Files selected for processing (5)
.ai-bot/feedback-workflow.md.ai-bot/instructions.md.ai-bot/new-ticket-workflow.mdAGENTS.mdCLAUDE.md
🛑 Comments failed to post (5)
.ai-bot/feedback-workflow.md (2)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a top-level heading.
markdownlintreports MD041 because the document starts with prose instead of an H1.🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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 @.ai-bot/feedback-workflow.md at line 1, Add a top-level H1 heading at the beginning of the feedback workflow document before the existing prose, using a concise title that describes the document and satisfies markdownlint MD041.Source: Linters/SAST tools
25-29: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the shell-command fence and surround it with blank lines.
Use
```bashand add the blank lines required by markdownlint.🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 25-25: 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 @.ai-bot/feedback-workflow.md around lines 25 - 29, Update the shell-command code fence in the documented workflow to use the bash language label (` ```bash `) and add blank lines immediately before and after the fenced block to satisfy markdownlint.Source: Linters/SAST tools
.ai-bot/instructions.md (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the markdownlint violations.
Add a top-level heading, specify
bashfor the first two fences, and add blank lines around every fenced block.Also applies to: 13-15, 19-21, 24-26, 29-31
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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 @.ai-bot/instructions.md at line 1, Update the markdown in instructions.md to satisfy markdownlint: add a top-level heading, label the first two fenced code blocks with bash, and insert blank lines immediately before and after every fenced block, including the sections referenced by the review.Source: Linters/SAST tools
.ai-bot/new-ticket-workflow.md (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the markdownlint violations.
Add a top-level heading, label the command fence as
bash, and surround it with blank lines.Also applies to: 22-27
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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 @.ai-bot/new-ticket-workflow.md at line 1, Add a top-level heading to the workflow document, add blank lines before and after the command block, and label its fenced code block as bash to resolve the markdownlint violations.Source: Linters/SAST tools
AGENTS.md (1)
26-30: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Explicitly require Git operations from the installer root.
“Never
cdinto submodule directories” does not prevent commands such asgit -C base/...from operating on a component repository. Add an explicit rule to run all Git commands from the installer root.As per coding guidelines, Git operations must run from the installer root, never inside submodules.
🤖 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 `@AGENTS.md` around lines 26 - 30, Update the “Submodules (READ ONLY)” guidance in AGENTS.md to explicitly require all Git commands to be executed from the installer root, never from within submodule directories; clarify that commands such as git -C base/... are also prohibited.Sources: Coding guidelines, Learnings
amej
left a comment
amej
left a comment
There was a problem hiding this comment.
As author, I have captured my views here. This exercise is becoming a tradeoff between thorough knowledge available to coding agents and the best practice of open standard AGENTS.md
AGENTS.md had picked up merge artifacts on the PR branch: a garbled Quick Start/Common Commands boundary, a missing Architecture heading, and a duplicate "Key Scripts" list repeated after the CI Workflows table. Restructure the file to fix all three. Also address eranco74's outstanding PR osac-project#396 review comments: - Drop /path/to/osac-installer placeholders; commands are cwd-relative in both AGENTS.md and CLAUDE.md (Bash Tool section) - Document the sha-XXXXXXX image tag format in the submodule notes Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Ameya Sathe <asathe@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
sync-charts refreshes submodule pins into chart dependencies and should run before make install*, not immediately before make uninstall where it was previously listed — it has no relation to teardown. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Ameya Sathe <asathe@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
The docs restructuring in this PR silently dropped several lines originally authored by others on main, with no replacement: - AGENTS.md: local-dev submodule sync guidance (auto-sync behavior, dirty pointers expected), pull-secret/AAP-license file locations, and the ca-bundle/trust-manager ownership note - .ai-bot/feedback-workflow.md: the "Session Artifacts" section (session-context.md, comment-responses.json) - .ai-bot/instructions.md: the detailed "Repository Structure" tree Restore all of it. The Repository Structure tree's base/ submodule list is updated to discover submodules dynamically (git submodule status) rather than hardcoding names, consistent with this PR's stated goal elsewhere in the docs. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Ameya Sathe <asathe@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Drop /path/to/osac-installer placeholders in feedback-workflow, instructions, and new-ticket-workflow so validation matches AGENTS.md and eranco74 review feedback. Assisted-by: cursor Signed-off-by: Ameya Sathe <asathe@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Use values/<env>/values.yaml and "Deploy to OpenShift" in Common Commands; Quick Start keeps the concrete development example. Assisted-by: cursor Signed-off-by: Ameya Sathe <asathe@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Validation gates use bare make helm-validate so all values envs are covered; single-env dry-runs remain via make helm-template. Assisted-by: cursor Signed-off-by: Ameya Sathe <asathe@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
amej
left a comment
amej
left a comment
There was a problem hiding this comment.
whether to use helm template render or helm validate for all environments was part I contemplated about.
I moved from cwd path to relative path on coderabbit's suggestion , then moved back to relative path on eran's suggestion.
Quick commands contains startup commands for dev environment.
But common section generalizes those commands .
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amej, eranco74 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
Re-triggered failed runs:
|
3 participants
Summary
make helm-lint,make helm-validate).ai-bot/workflow docs (feedback, instructions, new-ticket) for Helm validation/path/to/osac-installerplaceholders with cwd-relative commands, documented thesha-XXXXXXXimage tag format, movedmake sync-chartsbefore the deploy commands (it's a pre-deploy step, not teardown)Jira
https://redhat.atlassian.net/browse/OSAC-2402
Test plan
yamllint --strict .,pre-commit run --all-files,make helm-lint,make helm-validate)🤖 Generated with Claude Code