feat: add Claude skills and CLAUDE.md for AI-assisted development#190
feat: add Claude skills and CLAUDE.md for AI-assisted development#190louisliu2048 merged 2 commits intomainfrom
Conversation
Add `.claude/skills/` with two skills: - `review.md`: Thorough code review skill checking Rust quality, XLayer conventions, security, and test coverage - `commit.md`: Conventional commit message generation skill Add `CLAUDE.md` at repo root with project overview, key commands, repo structure, and coding standards for Claude Code to follow. Update `.gitignore` to allow `.claude/skills/` to be committed while keeping other local Claude settings private. Closes #189 Co-authored-by: Vui-Chee <Vui-Chee@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — docs/config-only PR with no production code impact; the inline comments note minor fixups for the gitignore patterns and review skill accuracy.
Extended reasoning...
Overview
This PR adds four files to support AI-assisted development: CLAUDE.md (project instructions), .claude/skills/commit.md (conventional commit skill), .claude/skills/review.md (code review skill), and a .gitignore update to track the skill files. No Rust source code, tests, or production configuration is modified.
Security risks
None. The PR contains only markdown documentation and a gitignore change. No code execution paths, dependencies, authentication logic, or sensitive data handling is affected.
Level of scrutiny
Low scrutiny is appropriate. These are AI instruction/configuration files with no runtime impact. The CLAUDE.md accurately reflects the repository structure (crates, docs/subguides, just commands). The commit skill follows standard conventional commits conventions. The review skill provides reasonable review guidelines.
Other factors
Two bugs were identified and posted as inline comments: (1) The .gitignore negation patterns are ineffective because .claude ignores the directory itself rather than its contents, preventing Git from descending into it. This only affects future skill file additions since the current files are tracked by this commit. The fix is straightforward (use .claude/* with proper negation layering). (2) The review skill references a non-existent EthApiExt pattern and omits two of the six subguides listed in CLAUDE.md. Both issues are minor and don't warrant blocking the PR — they're easily fixed in a follow-up. The overall content is accurate and useful.
| !.claude/skills/ | ||
| !.claude/skills/*.md |
There was a problem hiding this comment.
🔴 The .claude pattern on line 2 ignores the directory itself (not just its contents), which prevents Git from descending into it — making the negation patterns on lines 3-4 completely ineffective. Any new files added to .claude/skills/ will be ignored and require git add -f. Additionally, even if line 2 is fixed to .claude/*, the !.claude/skills/ negation unignores the entire directory (not just .md files), so the correct pattern needs four lines:
.claude/*
!.claude/skills/
.claude/skills/*
!.claude/skills/*.md
Extended reasoning...
How Git handles directory-level ignores
When .gitignore contains .claude (without a trailing slash or wildcard), Git ignores the directory entry itself. As an optimization, Git will not descend into an ignored directory at all, which means any negation patterns for paths inside that directory (like !.claude/skills/ or !.claude/skills/*.md) are never evaluated. This is documented Git behavior and is the root cause of the bug.
Step-by-step proof
Consider the current .gitignore patterns and a new file .claude/skills/new-skill.md:
- Git encounters
.claude/directory and checks ignore rules - The
.claudepattern on line 2 matches the directory itself → Git marks it as ignored - Since the directory is ignored (not just its contents), Git skips traversal entirely
- The negation patterns
!.claude/skills/and!.claude/skills/*.mdon lines 3-4 are never reached - Result:
.claude/skills/new-skill.mdis ignored
This was independently verified by multiple reviewers using git check-ignore -v .claude/skills/commit.md in fresh repositories, all confirming the file is ignored by the .claude rule.
Why existing files are tracked
The .claude/skills/commit.md and .claude/skills/review.md files introduced in this PR are currently tracked because they were committed in the same changeset as the .gitignore modification. Once files are tracked, .gitignore rules do not affect them. However, any new skill files added in the future would be silently ignored, requiring contributors to use git add -f — a confusing experience that defeats the purpose of these patterns.
The second issue: overly broad negation
Even if the first bug is fixed by changing .claude to .claude/* (ignoring contents rather than the directory), a subtler issue remains. The pattern !.claude/skills/ unignores the entire skills/ directory. Since .claude/* only matches direct children of .claude/ and does not match files nested deeper (like .claude/skills/anything), once the directory is unignored, all files inside it become trackable — not just .md files. The !.claude/skills/*.md pattern on line 4 becomes redundant because nothing is ignoring non-.md files in that directory in the first place.
Recommended fix
The correct four-line pattern that (a) allows Git to traverse .claude/ and (b) restricts tracked files to only .md files in skills/ is:
.claude/*
!.claude/skills/
.claude/skills/*
!.claude/skills/*.md
This works because: .claude/* ignores all direct children → !.claude/skills/ re-includes the skills directory so Git traverses it → .claude/skills/* ignores everything inside skills → !.claude/skills/*.md re-includes only .md files.
| ### 3. Check XLayer-Specific Conventions | ||
|
|
||
| - **Upstream compatibility**: Changes must not break upstream Reth compatibility; extensions should use trait implementations, not modifications to upstream types | ||
| - **Component isolation**: Changes in `crates/` should be isolated to their crate; cross-crate dependencies should be minimal and intentional | ||
| - **Database changes** (if any): Must follow `docs/subguides/DATABASE_GUIDE.md` — new tables need proper `Compress`/`Decompress` implementations | ||
| - **RPC extensions** (if any): Must follow `docs/subguides/EXTENSION_GUIDE.md` — use the `EthApiExt` pattern | ||
| - **Chainspec changes** (if any): Must follow `docs/subguides/CHAINSPEC_EVM_GUIDE.md` | ||
| - **Node builder changes** (if any): Must follow `docs/subguides/NODE_BUILDER_GUIDE.md` | ||
|
|
||
| ### 4. Check Security | ||
|
|
There was a problem hiding this comment.
🟡 The review skill has two inaccuracies in its XLayer-Specific Conventions section: (1) Line 33 references a non-existent EthApiExt pattern for RPC extensions — the codebase uses OpEthApi and the EXTENSION_GUIDE.md describes RpcModuleBuilder; (2) Only 4 of 6 subguides are listed, missing ENGINE_CONSENSUS_GUIDE.md and PAYLOAD_BUILDER_GUIDE.md which are correctly listed in CLAUDE.md. Consider fixing the pattern name and adding the two missing guide checks for consistency.
Extended reasoning...
The EthApiExt reference is fabricated
Line 33 of review.md says RPC extensions should "use the EthApiExt pattern." However, grepping the entire repository for EthApiExt returns zero results. The actual RPC extension pattern used in the codebase is OpEthApi (in crates/rpc/src/lib.rs), and the EXTENSION_GUIDE.md itself describes RpcModuleBuilder at line 97 as the pattern for adding custom RPC endpoints. The EthApiExt name appears to be hallucinated.
Missing two subguide checks
The XLayer-Specific Conventions section (lines 28-35) lists checks against 4 guides: DATABASE_GUIDE.md, EXTENSION_GUIDE.md, CHAINSPEC_EVM_GUIDE.md, and NODE_BUILDER_GUIDE.md. However, the docs/subguides/ directory contains 6 guides, and CLAUDE.md (lines 49-54) correctly lists all 6, including ENGINE_CONSENSUS_GUIDE.md and PAYLOAD_BUILDER_GUIDE.md. The review skill is inconsistent with the project instructions it was introduced alongside.
Step-by-step proof
grep -r EthApiExtacross the repository returns no matches — the pattern does not exist.- Reading docs/subguides/EXTENSION_GUIDE.md line 97 shows the actual pattern is RpcModuleBuilder.
ls docs/subguides/shows 6 files: CHAINSPEC_EVM_GUIDE.md, DATABASE_GUIDE.md, ENGINE_CONSENSUS_GUIDE.md, EXTENSION_GUIDE.md, NODE_BUILDER_GUIDE.md, PAYLOAD_BUILDER_GUIDE.md.- CLAUDE.md lines 49-54 lists all 6 guides.
- review.md lines 32-35 lists only 4, omitting ENGINE_CONSENSUS_GUIDE.md and PAYLOAD_BUILDER_GUIDE.md.
Impact
Since this is an AI instruction file, the practical impact is limited. Claude also has CLAUDE.md in context (which lists all 6 guides correctly), so it would likely self-correct. However, the fabricated EthApiExt reference could cause Claude to flag valid code or recommend a non-existent pattern during reviews. The missing guide checks mean the /review skill won't explicitly prompt checking engine/consensus or payload builder changes against their guides — relevant given the project has crates/builder/ and flashblocks support.
Addressing the refutation about triviality
One verifier argued the missing guides are too trivial to report since CLAUDE.md is also in context. While the impact is indeed minor, these are new files in this PR with an internal inconsistency — the fix is trivial (add two lines and fix one pattern name), and catching it now prevents the inconsistency from persisting. Another verifier flagged this as a potential duplicate of bug_008; this merged report covers both the EthApiExt issue and the missing guides as a single consolidated finding.
Suggested fix
On line 33, replace EthApiExt with RpcModuleBuilder. After line 35, add checks for ENGINE_CONSENSUS_GUIDE.md and PAYLOAD_BUILDER_GUIDE.md to match CLAUDE.md.
* main: (25 commits) fix: bump quinn-proto to 0.11.14 to patch CVE-2026-31812 DoS vuln (#183) pre-job authorization (#193) fix: trigger review skill failed to ack (#192) feat: trigger skill review separately (#191) feat: add Claude skills and CLAUDE.md for AI-assisted development (#190) rename ext (#185) supply workflow for claude (#184) feat(builder): incremental trie cache optimization for flashblocks state root (#163) chore(flashblocks-rpc): migrate op-reth flashblocks into xlayer-reth (#175) Feat(chainspec): adding xlayer-devnet chainspec (#167) chore(builder): flatten flashblocks builder, remove unnecessary trait interfaces (#172) rpc: remove unnecessary trait bounds and dependencies from XlayerRpcExtApiServer impl (#171) fix fmt in bin/tools/gen_genesis.rs (#170) fix(builder): Resolve bugs on upstream flashblocks timing scheduler (#169) Feat(tools): Add a tool to generate a custom genesis file based on a template and existing chain data (#159) feat(flashblocks): Add flashblocks sequence persistence logic on RPC and sequence replay flashblock builder (#162) chore(builder): remove unused custom-engine-api feature flag in tests (#168) fix: p2p test hang due to hang on port (#165) fix: update testcontainers to v0.27.0 to remediate CVE-2025-62518 (#164) chore(builder): further clean up builder crate (#161) ...
2 participants
Add Claude Code skills and project instructions to improve AI-assisted development.
CLAUDE.md: project overview, commands, and coding standards for Claude.claude/skills/review.md: XLayer-aware code review skill (/review).claude/skills/commit.md: conventional commit generation skill (/commit)Closes #189