NO-JIRA: Refactor AGENTS.md to be minimal with references#587
Conversation
Also folds the test-standards skill into a reference. This change is a slight optimisation to context usage, as well as making the test-standards reference available to agents other than Claude.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@mdbooth: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
WalkthroughDocumentation is reorganized to consolidate developer guidance. Four new reference documents are added under ChangesDocumentation Reorganization
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/reference/code-structure.md:
- Around line 36-37: Update the two bullet lines under the controllers section
to hyphenate compound modifiers: change the descriptions for "Machine Sync
Controller" and "MachineSet Sync Controller" so they read "machine-related
resources" and "machineset-related objects" respectively, ensuring the modifiers
are hyphenated while leaving the controller names unchanged.
🪄 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: 0acf2451-a54d-4ae2-894f-7a338be80742
📒 Files selected for processing (7)
.agents/reference/code-structure.md.agents/reference/style-guide.md.agents/reference/tasks.md.agents/reference/testing.md.claude/skills/deep-review/SKILL.md.claude/skills/test-standards/SKILL.mdAGENTS.md
💤 Files with no reviewable changes (1)
- .claude/skills/test-standards/SKILL.md
| - **Machine Sync Controller** (`pkg/controllers/machinesync/`) - Synchronizes individual machine related resources between APIs | ||
| - **MachineSet Sync Controller** (`pkg/controllers/machinesetsync/`) - Synchronizes machineset related objects between APIs |
There was a problem hiding this comment.
Hyphenate compound modifiers.
Lines 36 and 37 use "machine related" and "machineset related" as compound modifiers before nouns. According to English style conventions, these should be hyphenated: "machine-related resources" and "machineset-related objects".
📝 Proposed fix
-- **Machine Sync Controller** (`pkg/controllers/machinesync/`) - Synchronizes individual machine related resources between APIs
+- **Machine Sync Controller** (`pkg/controllers/machinesync/`) - Synchronizes individual machine-related resources between APIs
-- **MachineSet Sync Controller** (`pkg/controllers/machinesetsync/`) - Synchronizes machineset related objects between APIs
+- **MachineSet Sync Controller** (`pkg/controllers/machinesetsync/`) - Synchronizes machineset-related objects between APIs📝 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.
| - **Machine Sync Controller** (`pkg/controllers/machinesync/`) - Synchronizes individual machine related resources between APIs | |
| - **MachineSet Sync Controller** (`pkg/controllers/machinesetsync/`) - Synchronizes machineset related objects between APIs | |
| - **Machine Sync Controller** (`pkg/controllers/machinesync/`) - Synchronizes individual machine-related resources between APIs | |
| - **MachineSet Sync Controller** (`pkg/controllers/machinesetsync/`) - Synchronizes machineset-related objects between APIs |
🧰 Tools
🪛 LanguageTool
[grammar] ~36-~36: Use a hyphen to join words.
Context: ...ync/`) - Synchronizes individual machine related resources between APIs - **Machi...
(QB_NEW_EN_HYPHEN)
[grammar] ~37-~37: Use a hyphen to join words.
Context: ...hinesetsync/`) - Synchronizes machineset related objects between APIs ### Shared...
(QB_NEW_EN_HYPHEN)
🤖 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/reference/code-structure.md around lines 36 - 37, Update the two
bullet lines under the controllers section to hyphenate compound modifiers:
change the descriptions for "Machine Sync Controller" and "MachineSet Sync
Controller" so they read "machine-related resources" and "machineset-related
objects" respectively, ensuring the modifiers are hyphenated while leaving the
controller names unchanged.
|
@mdbooth: 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. |
|
this is posted by claude, reviewed by me 🎉 I've reviewed this and the instinct is right — the old AGENTS.md was 184 lines, already over the recommended ≤150 line guideline. ETH Zurich research found that bloated context files hurt agent performance (20%+ increased inference costs with no accuracy benefit). A lean root file is the right shape. I have a few concerns with the approach though. I've left inline comments on the specific areas. |
| @@ -0,0 +1,56 @@ | |||
| # Code Structure | |||
There was a problem hiding this comment.
`.agents/` is not a standard directory — consider hierarchical `AGENTS.md` files instead
The established convention for agent instructions is hierarchical `AGENTS.md` files placed in subdirectories. Agents auto-read the nearest `AGENTS.md` in the directory tree — the closest one takes precedence, and each subdirectory can ship tailored instructions. The OpenAI repo has 88 of them. This is supported natively by Claude Code, Codex, Cursor, Copilot, Jules, and others, and is now governed by the Agentic AI Foundation under the Linux Foundation.
There is a community proposal for a `.agent` directory (singular, different structure), but it's at the discussion stage and not adopted by any tool. The `.agents/reference/` directory used here doesn't match the standard, doesn't match the proposal, and won't be auto-discovered by any agent tooling.
These files will only be read if an agent actively decides to follow the links in the root AGENTS.md — which is not guaranteed, especially for quick fixes or tasks where the agent doesn't think to check conventions.
Suggestion: Place this content in `AGENTS.md` files in relevant subdirectories instead (e.g., `pkg/AGENTS.md`, `e2e/AGENTS.md`). This gets the same "thin root, detailed when needed" outcome while using the standard mechanism that tools already auto-discover.
| @@ -1,121 +0,0 @@ | |||
| --- | |||
There was a problem hiding this comment.
Removing this skill loses the auto-trigger on `_test.go` files
This skill had an auto-trigger: it fired automatically when agents worked on `_test.go` files, injecting testing conventions into context at exactly the right time. Moving the content to `.agents/reference/testing.md` and deleting the skill removes that trigger.
Now an agent writing or modifying tests will only see the testing guidance if it independently decides to read the reference file — which isn't guaranteed, especially for quick test fixes.
The `deep-review` skill was updated to point at the new location, so reviews are covered. But unguided work (writing new tests, fixing test failures) loses the automatic context injection.
Suggestion: Either keep this skill (pointing at the new reference file location), or ensure the testing guidance ends up in a subdirectory `AGENTS.md` that tools auto-discover.
| ## Project Overview | ||
|
|
||
| The Cluster CAPI Operator manages the installation and lifecycle of Cluster API components on OpenShift clusters. It serves as a bridge between OpenShift's Machine API (MAPI) and the upstream Cluster API (CAPI), providing forward compatibility and migration capabilities. | ||
|
|
There was a problem hiding this comment.
These links won't be reliably followed by agents
The root AGENTS.md is auto-loaded into context by Claude Code, Codex, Cursor, Copilot, and others. But these linked files are regular markdown files — an agent has to actively decide to read them. That decision depends on the agent judging that the reference is relevant to the current task, which won't happen consistently.
For example: with the old inline AGENTS.md, "never use `time.Sleep()`, use `Eventually`" was always in context during test work. Now it's behind a link the agent may or may not follow. "Always applied" vs "usually applied" is where regressions show up.
The standard approach here is subdirectory `AGENTS.md` files (see comment on `.agents/reference/`), which tools auto-discover without the agent needing to make a judgment call.
3 participants
Also folds the test-standards skill into a reference.
This change is a slight optimisation to context usage, as well as making the test-standards reference available to agents other than Claude.
Summary by CodeRabbit
Note: This release contains internal documentation updates with no changes to user-facing features or functionality.