Conversation
Mined 6 months of merged PR review comments (human reviewers only) for conventions that recur across multiple PRs/reviewers but aren't already documented in CONTRIBUTING.md/CodingStyle.md, and folded them in: - Doxygen comments expected on new public functions/methods. - API-CHANGELOG.md requirement is repo-wide, not RPC-directory-scoped. - Shared test setup belongs in jtx/, not copy-pasted across test files. - Amendment requirement test and a design-considerations checklist, sourced from the internal "Considerations for Future Amendments" doc. - Amendment gating shape (single enabled/disabled block), per-amendment flag naming, and UNREACHABLE/LCOV_EXCL pairing, all recurring asks in tx/vault/invariant PR reviews.
…and READMEs Round 2 of PR-review mining: found two subsystem-specific, unrecorded conventions (pseudo-account auth/freeze exemption in vault/lending transactors; SLE-type assertion in ledger helpers) that warrant their own nested AGENTS.md rather than more root-level bullets. Also added thin AGENTS.md pointer files (with CLAUDE.md symlinks, matching the existing convention) for subsystems that already have a substantial README.md but no AGENTS.md — agent tooling auto-loads AGENTS.md when working in a directory but not README.md, so those conventions were otherwise invisible to an agent that didn't think to look.
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
There was a problem hiding this comment.
Doc-only change adding/updating AGENTS.md (and matching CLAUDE.md symlinks) files. Checked all new relative links (to repo-level AGENTS.md, README.md, docs/consensus.md) against each file's actual directory depth and they all resolve correctly. Content additions (jtx test-setup rule, Doxygen requirement, repo-wide API-CHANGELOG rule, amendment-gating guidance, pseudo-account exemption note, SLE-helper assertion note) are consistent with the stated conventions and don't contradict existing docs. No correctness, security, or wiring issues found in the changed lines.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Docs-only change adding/updating AGENTS.md files across the repo to codify recurring PR review feedback for AI coding agents. Checked all new relative markdown links (root AGENTS.md, README.md, docs/consensus.md) against each file's actual directory depth — all path traversals (../../../, ../../../../, etc.) resolve correctly. The new API-Changelog anchor reference (AGENTS.md#api-changelog) matches the corresponding heading. CLAUDE.md symlinks are consistent with the existing repo convention. No correctness, security, or process-config issues found; nothing in this diff touches behavior-affecting code or repo-wide gating/config files.
There was a problem hiding this comment.
🟡 Changes recommended
Two newly added guidance statements are inconsistent with established code patterns/behavior (pseudo-account freeze/deep-freeze handling and std::expected/std::unexpected usage), which could mislead future changes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates and expands AGENTS.md guidance across the repo to capture recurring review expectations (especially around API changelog discipline, amendments, and subsystem design pointers) so AI coding agents automatically load the right conventions when working in those directories.
Changes:
- Promotes repo-wide API changelog + Doxygen expectations into the root
AGENTS.mdand clarifies how RPC changes fit that rule. - Expands
src/libxrpl/tx/AGENTS.mdwith amendment “when required” guidance plus concrete gating conventions. - Adds “thin pointer”
AGENTS.mdfiles in several subsystems to direct agents to existing design docs (README.md,docs/consensus.md).
File summaries
| File | Description |
|---|---|
AGENTS.md |
Adds shared-test placement guidance, clarifies Doxygen expectations, and adds an explicit repo-wide API changelog rule. |
src/xrpld/rpc/AGENTS.md |
Clarifies that RPC behavior changes fall under the repo-wide API changelog requirement and points to the RPC README. |
src/xrpld/peerfinder/AGENTS.md |
Adds a pointer to repo-level guidance and the peerfinder design README. |
src/xrpld/overlay/AGENTS.md |
Adds a pointer to repo-level guidance and overlay design README. |
src/xrpld/app/ledger/AGENTS.md |
Adds a pointer to repo-level guidance and ledger subsystem README. |
src/xrpld/app/consensus/AGENTS.md |
Adds pointers to repo-level guidance, local README, and docs/consensus.md. |
include/xrpl/shamap/AGENTS.md |
Adds a pointer to repo-level guidance and SHAMap README. |
include/xrpl/nodestore/AGENTS.md |
Adds a pointer to repo-level guidance and NodeStore README. |
include/xrpl/consensus/AGENTS.md |
Adds pointers to repo-level guidance, local README, and docs/consensus.md. |
src/libxrpl/tx/AGENTS.md |
Adds detailed amendment criteria and conventions for structuring amendment-gated code paths. |
src/libxrpl/tx/transactors/AGENTS.md |
Adds transactor-specific guidance about pseudo-accounts and preferring invariants over duplicated per-transactor checks. |
src/libxrpl/ledger/helpers/AGENTS.md |
Adds guidance on asserting SLE type/non-nullness and recommended error-handling style for helpers. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| See the repo-level [AGENTS.md](../../../../AGENTS.md) for general guidance. | ||
|
|
||
| A helper that takes an `SLE`/`std::shared_ptr<SLE const>` should `XRPL_ASSERT` that it's non-null and of the expected ledger-entry type at entry, and keep a real runtime check/error-return alongside the assert (asserts compile out in release builds). Don't invent a new error-handling idiom for this (e.g. `std::unexpected`) — return the existing `tec`/`ter`/`tef` code used elsewhere in the codebase. |
| @@ -0,0 +1,3 @@ | |||
| # AGENTS.md — peerfinder | |||
|
|
|||
| See the repo-level [AGENTS.md](../../../AGENTS.md) for general guidance and [README.md](./README.md) for the peer-discovery design. | |||
There was a problem hiding this comment.
Could you please explain why do we need two almost identical files in every directory? I don't really know best practice here, but it looks like duplication
There was a problem hiding this comment.
It just points to the README in each of these subfolders, since the README already covers all the data needed and there's no need for separate agent-specific documentation. This seems in line with your earlier comment suggesting that we have contributor-focused documentation instead of just AI-focused documentation.
There was a problem hiding this comment.
The project-map.md file basically describes and links to appropriate source for everything. We don't really need to put these files in each sub-dir.
| See the repo-level [AGENTS.md](../../../AGENTS.md) for general build/test/style guidance. | ||
|
|
||
| Any change to transaction-processing behavior must be gated behind an amendment. New amendments (and fixes, i.e. `fix*` amendments) are added to [`include/xrpl/protocol/detail/features.macro`](../../../include/xrpl/protocol/detail/features.macro), as an `XRPL_FEATURE(...)` or `XRPL_FIX(...)` entry added to the top of the list (the list is kept in reverse chronological order). Once the pre-amendment code path for a retired amendment is removed, move its entry to `XRPL_RETIRE_FEATURE(...)`/`XRPL_RETIRE_FIX(...)` instead of deleting it. | ||
| ## When an amendment is required |
There was a problem hiding this comment.
Maybe instead we should have a contributing guidelines doc? Such rules may be useful not only for agents but for new contributors
There was a problem hiding this comment.
No objections here, we can make it a readme that the agents.md points to like what other files in this PR do. That should be a separate PR though, because I'd like this in first. Fine with this info migrating later, but I'd rather not have perfect be the enemy of good here.
|
|
||
| When a file or function checks more than one amendment, name local enablement booleans per-amendment (e.g. `fix340Enabled` for `fixCleanup3_4_0`), not a generic `fixEnabled` — it becomes ambiguous once a second amendment is checked in the same scope. | ||
|
|
||
| Only use `UNREACHABLE` for genuinely impossible paths, not to avoid writing a test for one that's reachable but rare. When a branch marked `UNREACHABLE` is excluded from coverage, wrap it in `LCOV_EXCL_START`/`LCOV_EXCL_STOP`. |
There was a problem hiding this comment.
This rule is not related to amendment
Team decided AGENTS.md content should live in the narrowest folder it applies to, rather than the broadest, to keep context small for agents working elsewhere in the repo. Restructure round 1/2's additions accordingly, with no change in the rules themselves: - Move the repo-wide API-CHANGELOG.md rule out of root AGENTS.md and into rpc/AGENTS.md (its natural home), with a one-line pointer left in tx/AGENTS.md for the signing-behavior case. - Move the amendment-gating *code style* rules (single enabled/disabled block, per-amendment flag naming, UNREACHABLE/LCOV_EXCL pairing) from tx/AGENTS.md down into transactors/AGENTS.md, since all cited PR evidence was in transactor code, not tx/-level files. - Move the "shared test setup belongs in jtx/" rule out of root AGENTS.md into a new src/test/AGENTS.md (+ CLAUDE.md symlink). - Trim the now-duplicated amendment sentence out of root's Architecture bullet in favor of a pointer to tx/AGENTS.md.
… mvadari/codify-agents-md
There was a problem hiding this comment.
This is a documentation-only change adding/expanding AGENTS.md files (plus CLAUDE.md symlinks) across the repo. I checked every new/modified relative link (root AGENTS.md, docs/consensus.md, README.md pointers, cross-references between tx/AGENTS.md, transactors/AGENTS.md, and rpc/AGENTS.md) and all resolve to the correct depth given each file's location. The symlinks are all simple 'AGENTS.md' relative symlinks matching the stated existing convention, and each target AGENTS.md exists alongside it. No code, security, or correctness issues apply since no executable code is touched; content changes are guidance text sourced from cited PR review history per the description, which is outside the scope of an automated correctness review. No issues found worth flagging.
- Build section now recommends the Nix devshell rather than requiring it; contributors can use their own toolchain/build flow. - Move the directory-by-directory architecture map out of AGENTS.md into a new top-level ARCHITECTURE.md, with a pointer left behind. - Add a short note asking contributors to keep AGENTS.md/README/ ARCHITECTURE.md files current when they change what those docs cover. - ledger/helpers/AGENTS.md also needs the amendment-gating shape, per-amendment flag naming, and UNREACHABLE/LCOV_EXCL pairing notes (previously only in transactors/AGENTS.md) — the vault/lending helper files these were evidenced on live in ledger/helpers/, not tx/transactors/.
There was a problem hiding this comment.
This is a documentation-only MR adding/editing AGENTS.md files. All relative markdown links check out (correct ../ depth for every new/modified file), and the added guidance is generally consistent with the MR's stated rationale. One notable inconsistency: the amendment-gating and UNREACHABLE/LCOV guidance is duplicated verbatim in both the new ledger/helpers/AGENTS.md and tx/transactors/AGENTS.md, which cuts against the MR's own stated principle that each rule should live in the narrowest directory it applies to.
… section - ledger/helpers/AGENTS.md: std::expected<..., TER>/std::unexpected(tec*) is already the established idiom here (confirmed in AMMHelpers.cpp, CredentialHelpers.cpp), not something to avoid; reword so the rule doesn't read as discouraging it. - transactors/AGENTS.md: the UNREACHABLE/LCOV_EXCL coverage rule isn't amendment-specific, so give it its own heading instead of nesting it under "Gating amendment-dependent code". - Root AGENTS.md: comments should explain why, not what/how, since the code already shows that. - Drop the pseudo-account exemption rule from transactors/AGENTS.md — needs refining before it's codified. Addresses PR review comments: #8198 (comment) #8198 (comment)
There was a problem hiding this comment.
Documentation-only change (AGENTS.md/ARCHITECTURE.md additions). Cross-file links and relative paths all resolve correctly, and the new content generally matches the stated goal of scoping rules to the narrowest applicable directory. Found two places where the actual file content doesn't match the MR's own description/design intent: ledger/helpers/AGENTS.md duplicates transactor-specific amendment-gating/UNREACHABLE guidance that the MR description says is scoped to transactors/ only, and transactors/AGENTS.md is missing the pseudo-account (Vault/LoanBroker/AMM) requireAuth/freeze exemption rule that the MR description claims was added there.
godexsoft
left a comment
godexsoft
left a comment
There was a problem hiding this comment.
Looks fine to me for the first version. We can always improve later 👍
| @@ -0,0 +1,9 @@ | |||
| # Architecture | |||
There was a problem hiding this comment.
This needs to go in project-map.md file. Attaching mine:
project-map.md
There was a problem hiding this comment.
project-map.md file then links to other files like docs/consensus.md to deeper details. None of those files are actually loaded in context at startup. They are pulled as required. I have files containing details on architecture, components, features, etc.
There was a problem hiding this comment.
This feels like primarily a disagreement on file name, and seems to disagree with your desire on keeping context small (i.e. IMO this project map file should be split up into smaller files, one of them being this ARCHITECTURE.md). Can you clarify what you're looking for?
There was a problem hiding this comment.
It's not the name really. Since we will need to refer it in some repo level CLAUDE.md/AGENTS.md file, and we can use any name there.
It's about the purpose. So the project-map is supposed to contain short idea about how the project is structured. It is supposed to map the project tree itself. It is the go to place for AI Agent to find things. Hence the name. A map basically. It stays under 200 lines. It should always remain in context, so that the agent can look at the map and navigate. It should need to be read again and again. If we break this down into individual files, the agent will definitely miss (intentionally skip in most cases) reading connected/related pieces together.
The architecture.md contains the architecture of the software itself. Like the building blocks and how they interact. I am sharing my achitecture.md file here.
architecture.md
I had this linked into the project-map file but I removed it before sharing that file, it it had local paths and to avoid confusion.
There was a problem hiding this comment.
project-map file basically removes the need of individual indirection files in each sub-folder. I tried to find the blog post from Anthropic or OpenAI, where they suggest the usecase and benefits of project-map.md file and they use the same name. But couldn't find it now. Maybe they removed it after their source leak was attributed to the leaked project-map file XD
Not attached to the name though. It is just that architecture.md should define software, and project-map should define the project arrangement.
There was a problem hiding this comment.
Can you open a separate PR that adds those? I'd like to get the benefits from this PR in first, and these changes would be a big scope expansion (plus, I think it makes more sense for that PR to come from you).
There was a problem hiding this comment.
But that new PR will anyway remove all the agents.md files from all the sub dirs!
For this PR, I would suggest we put the project-map.md file in .agents/ folder. That's it. Remove everything else.
It already points to other important files. It points to Contributing.md file as well, which contains code guidelines. It also points to other coding and general guidelines files.
Any developer can just point their agents.md/claude.md to this project-map.md.
In next iterations, we can include more specific files. All of them will go in .agents/ folder.
If we do it any other way, we just add redundant linked files all over the repo. This way we get what we need, a repo level+ repo. specific common info. inside the repo. itself, without breaking anyone's personal setup. Since that goes through repo level AGENTS.md/CLAUDE.md files, which we really should not touch.
@kuznetsss What do you think?
There was a problem hiding this comment.
The sub-dir AGENTS.md files are all different than what's contained in the project-map - the project-map.md file doesn't contain information that is helpful for an agent wrt actually doing the work, it only points it to the right section of the code to get started.
There was a problem hiding this comment.
That's exactly what the project-map.md file is for. 'information helpful for actually doing the work' is something, we want the agent to read only when it is needed. We don't want to pollute the context with info about each module per say.
A-few lines in info about a feature is not going to help the agent much anyway. It will read the code and docs. And its better that it reads it on need basis. But we can anyway put such info at one place, like in architecture.md, workflow.md, feature.md file. That way the agent has info about the subject + dependencies.
There was a problem hiding this comment.
I think we should have a good default for everyone with a way to opt out. Turns out claude code has a way to exclude any file form automatic loading: https://code.claude.com/docs/en/memory#exclude-specific-claude-md-files. Maybe worth adding the link to CONTRIBUTING.md.
Because there is a way to opt out, I'm ok to merge this PR as it is and improve the structure in the next iteration.
My suggestion would be to put all the rules into a doc file per subdirectory, something like docs/conventions/dirs/subdirectory.md. And reference such file in subdirectory's AGENTS.md.
Such structure would help to avoid duplications between include and src subdirectories, keep all the rules in one place and store rules as docs, so humans could also read them.
| @@ -4,13 +4,13 @@ This file provides guidance to AI coding agents (Claude Code, and other AGENTS.m | |||
|
|
|||
There was a problem hiding this comment.
Instead of duplicating content here, we should just point to the relevant files. BUILD.md, CONTRIBUTING.md, others: rippled/docs/*.md. This doesn't need to sit it context. This should only be fetched when required.
There was a problem hiding this comment.
Any specific places I should point to? It's already pointing to those files when one exists and not duplicating existing documentation.
There was a problem hiding this comment.
The pointing will be per-heading:
Build: Build.md
Coding Guidlines: docs/CodingStyle.md
Please check the project-map.md file I attached in another comment.
…tion - The harness auto-loads every ancestor AGENTS.md (including root), so a nested file telling an agent to "see the repo-level AGENTS.md" is pure overhead — it can trigger a re-load/duplication-check/discard cycle, and some models just retain the duplicate content in context. Drop that pointer from every nested AGENTS.md; keep pointers to things that are NOT auto-loaded (README.md, docs/consensus.md, non-ancestor AGENTS.md like tx/ -> rpc/). - Root AGENTS.md's Build section is now a short index pointer to BUILD.md and docs/build/nix.md instead of restating Nix setup steps, per review feedback that AGENTS.md should mostly index other docs rather than duplicate them. Addresses PR review comments: #8198 (comment) #8198 (comment)
… mvadari/codify-agents-md
There was a problem hiding this comment.
Documentation-only PR (AGENTS.md conventions). The overall structure is sound and matches the stated "narrowest directory" design principle, with one exception: the new src/libxrpl/ledger/helpers/AGENTS.md duplicates the amendment-gating and UNREACHABLE/coverage guidance verbatim from src/libxrpl/tx/transactors/AGENTS.md, even though the PR description explicitly scopes that guidance to transactors (evidenced from vault/lending transactor PR reviews) and describes the helpers file as being only about SLE-type assertions.
| @@ -0,0 +1,7 @@ | |||
| # AGENTS.md — ledger/helpers | |||
|
|
|||
| A helper that takes an `SLE`/`std::shared_ptr<SLE const>` should `XRPL_ASSERT` that it's non-null and of the expected ledger-entry type at entry, and keep a real runtime check/error-return alongside the assert (asserts compile out in release builds) — the established idiom in this directory is `std::expected<..., TER>`, returning `std::unexpected(tec*)` on failure. Don't invent a new error-handling idiom for this. | |||
There was a problem hiding this comment.
Could we limit this to helpers where non-nullness and the entry type are actual preconditions? isPseudoAccount, for example, explicitly accepts null pointers and non-account entries and returns false. Adding the assertions described here would break that contract in Debug builds, even if we kept the runtime checks.
|
|
||
| See [README.md](./README.md) for basic `--unittest` invocation. | ||
|
|
||
| Shared test setup/helper code used by more than one test file belongs in `jtx/`, not copy-pasted across test files. |
There was a problem hiding this comment.
Could we scope this to shared transaction-building helpers rather than all shared test code? We already keep general utilities like SuiteJournal in src/test/unit_test/ and shared lending fixtures in src/test/app/lending/LoanTestBase.h. Avoiding copy-paste makes sense, but those helpers shouldn't have to move into jtx/ just because multiple tests use them.
|
|
||
| Prefer a single amendment-enabled block and a single disabled block over scattering `rules.enabled(...)` checks through a function, even if the two blocks are similar. When a file or function checks more than one amendment, name local enablement booleans per-amendment (e.g. `fix340Enabled` for `fixCleanup3_4_0`), not a generic `fixEnabled`. | ||
|
|
||
| Only use `UNREACHABLE` for genuinely impossible paths, not to avoid writing a test for one that's reachable but rare. When a branch marked `UNREACHABLE` is excluded from coverage, wrap it in `LCOV_EXCL_START`/`LCOV_EXCL_STOP`. |
There was a problem hiding this comment.
This is a general rule for the whole project and it is duplicated in src/libxrpl/tx/transactors/AGENTS.md.
There was a problem hiding this comment.
When we talked on Wednesday, you didn't want this to be in the top level of the project because it doesn't apply to Rust code. Can you confirm which you'd prefer?
There was a problem hiding this comment.
I think it should be in a document cpp_guidelines.md and AGENTS.md could reference that document.
There was a problem hiding this comment.
Or it can be in a single code_conventions.md file with the file/path filtering you shared earlier?
https://code.claude.com/docs/en/memory#path-specific-rules
Either way is fine.
There was a problem hiding this comment.
The UNREACHABLE/codecov stuff I'm fine with moving to a separate doc, but the amendment notes should stay here, since that doesn't apply to the whole repo.
There was a problem hiding this comment.
We could put them in amendments.md file. There will also be a features.md file with short desc. of features and links to XLS docs.
There was a problem hiding this comment.
That won't work if the agent is working on something that it doesn't think is an amendment.
There was a problem hiding this comment.
subdir/AGENTS.md could refer to the related directory doc which would contain directory specific rules or links to the other common docs. I would prefer to keep AGENTS.md as simple as possible and not duplicating docs for humans (not a blocker for this PR).
kuznetsss
left a comment
kuznetsss
left a comment
There was a problem hiding this comment.
I left some comments about duplications and structure, but those could be resolved in the next PR.
|
|
||
| Prefer a single amendment-enabled block and a single disabled block over scattering `rules.enabled(...)` checks through a function, even if the two blocks are similar. When a file or function checks more than one amendment, name local enablement booleans per-amendment (e.g. `fix340Enabled` for `fixCleanup3_4_0`), not a generic `fixEnabled`. | ||
|
|
||
| Only use `UNREACHABLE` for genuinely impossible paths, not to avoid writing a test for one that's reachable but rare. When a branch marked `UNREACHABLE` is excluded from coverage, wrap it in `LCOV_EXCL_START`/`LCOV_EXCL_STOP`. |
There was a problem hiding this comment.
You'll also want to mention SOMETIMES here. We tend to use it for situations that are rare.
For instance, values that should be positive - the code rejects non-positive values, but since the values are supplied by a peer it is technically possible for them to send us a negative value. Example:
bool isNegative = value.peek() < 0;
SOMETIMES(isNegative, "....")
if (isNegative)
{
JLOG(journal.debug(), "....");
return false;
}
There was a problem hiding this comment.
Since these AGENTS.md files discuss UNREACHABLE, XRPL_ASSERT, etc. multiple times, I'd highly recommend updating the external/antithesis-sdk/README.md with explanations of these defines, and then referencing that README.md in the agent files.
| New file placement and header levelization: see [CONTRIBUTING.md](./CONTRIBUTING.md#before-making-a-pull-request). Braces, whitespace, member order, and other conventions: see [docs/CodingStyle.md](./docs/CodingStyle.md). `XRPL_ASSERT`/`UNREACHABLE` contracts: see [CONTRIBUTING.md](./CONTRIBUTING.md#contracts-and-instrumentation). Commit messages: see [CONTRIBUTING.md](./CONTRIBUTING.md#good-commit-messages). | ||
| New file placement and header levelization: see [CONTRIBUTING.md](./CONTRIBUTING.md#before-making-a-pull-request). Braces, whitespace, member order, and other conventions: see [docs/CodingStyle.md](./docs/CodingStyle.md). `XRPL_ASSERT`/`UNREACHABLE` contracts: see [CONTRIBUTING.md](./CONTRIBUTING.md#contracts-and-instrumentation). Commit messages: see [CONTRIBUTING.md](./CONTRIBUTING.md#good-commit-messages). New public functions/methods need a Doxygen-style comment. | ||
|
|
||
| Comments should explain _why_, not _what_/_how_ — the code already shows that. Only describe what/how when the code itself would otherwise be confusing (a non-obvious workaround, a subtle invariant, a surprising constraint). |
There was a problem hiding this comment.
I'd also suggest expressing a preference here for comments that follow proper sentence format: start with a Capital letter and end with a period.
The codebase is very inconsistent and it's not worth spending dedicated time on cleaning them up by rewriting. However, it would be worth to ensure that new/updated comments are consistent.
High Level Overview of Change
Mines the last 6 months of merged-PR human review comments (bots excluded) for conventions that recur across multiple PRs/reviewers but aren't already documented in CONTRIBUTING.md/docs/CodingStyle.md, and folds them into
AGENTS.mdfiles so AI coding agents pick them up automatically. Each rule lives in the narrowest directory it applies to (not root), so agents working elsewhere don't load irrelevant context.Context of Change
AGENTS.md: Doxygen comments on new public functions/methods (genuinely repo-wide); trimmed the Architecture section's amendment mention to a pointer, since the detail now lives intx/AGENTS.md.src/libxrpl/tx/AGENTS.md: a precise "when is an amendment required" test and a design-considerations checklist (sourced from an internal amendment-writer doc); new/deleted invariant checks must be amendment-gated (consensus-safety, since an un-gated invariant risks validators disagreeing on a transaction's result).src/libxrpl/tx/transactors/AGENTS.md(new): amendment-gating code shape (single enabled/disabled block, not scattered checks), per-amendment flag naming (fix340EnablednotfixEnabled),UNREACHABLE/LCOV_EXCL_START/STOPpairing, pseudo-accounts (Vault/LoanBroker/AMM) being exempt fromrequireAuth/freeze checks as a class, and preferring one object-level invariant over duplicating a delta check per transactor. All evidenced in vault/lending transactor PR reviews specifically, so scoped here rather than the parenttx/AGENTS.md.src/libxrpl/ledger/helpers/AGENTS.md(new): helpers should assert the SLE type they're handed rather than inventing a new error-handling idiom.src/xrpld/rpc/AGENTS.md: theAPI-CHANGELOG.mdrequirement lives here (its natural home) rather than at root, since it's the primary place API-visible changes happen;tx/AGENTS.mdhas a one-line pointer to it for the signing-behavior case.src/test/AGENTS.md(new): shared test setup/helpers belong injtx/, not copy-pasted across test files.AGENTS.mdfiles (withCLAUDE.mdsymlinks, matching the existing convention) for subsystems that already have a substantialREADME.mdbut noAGENTS.md—overlay/,app/ledger/,shamap/,peerfinder/,nodestore/,app/consensus/,include/xrpl/consensus/, andrpc/. Agent tooling auto-loadsAGENTS.mdwhen working in a directory but notREADME.md, so those existing design docs were otherwise easy for an agent to miss.Every added rule cites the specific PR(s) where it recurred; I cross-checked each against CONTRIBUTING.md/docs/CodingStyle.md first to avoid duplicating what's already documented there.
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)