Skip to content

Add defensive CLAUDE.md, making AI (claude code) help enforce our AI PR rules#118681

Draft
Ivorforce wants to merge 1 commit into
godotengine:masterfrom
Ivorforce:add-claude-md
Draft

Add defensive CLAUDE.md, making AI (claude code) help enforce our AI PR rules#118681
Ivorforce wants to merge 1 commit into
godotengine:masterfrom
Ivorforce:add-claude-md

Conversation

@Ivorforce
Copy link
Copy Markdown
Member

@Ivorforce Ivorforce commented Apr 17, 2026

Note

AI Disclosure: The CLAUDE.md file was written by AI, as per my instructions. The PR description is written by me, by hand.

We're seeing more and more AI PRs popping up. Many AI contributors are unaware of our rules and guidelines. Some even let the AI run loose and don't double-check its contributions. This is a violation of our guidelines, and wastes our time with reviews of unmoderated (by the author) slop.

Thought process / proposed fix

I am proposing to bring the AI on our side, so to speak, and have it take part in enforcing our rules. In this PR, this is done for claude code.

The aim is notably not to help the AI to code, or understand the codebase - that is the responsibility of the AI and/or AI contributor. This PR (almost entirely) adds failsafes that help us, the Godot community, maintain a workable engine for everyone.

Concepts covered in CLAUDE.md:

  • Contribution gate: Contributors are asked to consent to the repository AI rules before being allowed to use an AI on it.
  • AI workflow: The AI is made aware of our contribution rules, such as the need to benchmark optimization changes, and helps enforce the rules by e.g. asking the user to test optimizations.
  • Summary of rules and guidelines: The AI has a summary of our most important rules (and links to more) so the changes at least somewhat adhere.

Effectiveness of this change

Before this change, the AI happily jumps on the task, no questions asked:

Before this PR
❯ Please improve the performance of adding children to Node.                                                                                                                     

⏺ Explore(Investigate Node add_child perf)
  ⎿  Done (43 tool uses · 60.6k tokens · 4m 3s)
  (ctrl+o to expand)

  Searched for 6 patterns, read 2 files (ctrl+o to expand)     
                                
⏺ Let me look at the cache rebuild and any existing PR context before proposing changes.
                                                      
  Searched for 1 pattern, read 1 file (ctrl+o to expand)     
                        
⏺ Before changing a hot Godot engine path, let me sketch what I found and propose a direction.                                                                                   
                                                      
  The terrain                                                                                                                                                                    
  - Fast path (default, p_force_readable_name=false) already got a 2x speedup in 2023 (stack-allocated char buffer). Remaining costs per add_child:                              
    - _validate_child_name: builds a String from the char32_t[] buffer, then constructs a StringName from that String (heap alloc + mutex + hash-table insert).                  
    - _add_child_nocheck: HashMap insert, cache append-or-dirty, NOTIFICATION_PARENTED, NOTIFICATION_CHILD_ORDER_CHANGED, and an emit_signal("child_order_changed") — the last   
  three fire unconditionally, every call.                                                                                                                                        
  - Slow path (p_force_readable_name=true, _generate_serial_child_name): each collision iteration allocates a String and a StringName just to probe the children map. Explicitly 
  marked "very slow".                                                                                                                                                            
                                                                                                                                                                                 
  Proposed changes (smallest → most invasive)                                                                                                                                    
                                                                                                                                                                                 
  1. Short-circuit can_push_back computation when cache is already dirty in _add_child_nocheck. Tiny win, trivially safe.                                                        
  2. Avoid the intermediate String in the fast-path name build by adding a StringName constructor that takes a (char32_t *, length, hash) directly — skips one heap alloc and one
   hash recompute per uniquely-named add. Most impact per byte of diff, but touches core/string/string_name.{h,cpp}.                                                             
  3. Probe without allocating in _generate_serial_child_name: add StringName::search(const String&) that returns empty if the name isn't already interned — lets the slow path
  skip StringName allocation on every failed attempt. Helps editor / tool code that uses p_force_readable_name=true.                                                             
                  
  Tradeoff: #2 and #3 add public API surface to StringName. #1 alone is unlikely to be measurable.                                                                               
                  
  Want me to go with #1 + #2 (the main runtime path), or include #3 too? Or narrower/broader?                                                                                    
                  
✻ Sautéed for 6m 48s

After this change, the AI consistently (10/10 times) gates the changes with an 'acceptance of terms' question.
It also asks the user to provide an existing issue, and proposes to benchmark.

After this PR
❯ Please improve the performance of adding children to Node.                                                                                                                     
                                                             
  Listed 1 directory (ctrl+o to expand)                                                                                                                                          
                                                                                                                                                                               
⏺ Before I can modify any file in this repo, CLAUDE.md requires you to read and accept the AI contributor checklist. Displaying it verbatim:                                     
                             
  ---                                                                                                                                                                            
  Checklist for AI contributors                                                                                                                                                  
                                                                                                                                                                                 
  Godot discourages AI-assisted contributions but permits them on these terms. Contributions made entirely by AI are prohibited outright. Full source: Godot Pull-Request        
  Guidelines. Read each item, then reply "I accept" to proceed.                                                                                                                  
                                                                                                                                                                                 
  1. Disclose AI involvement in the PR. When AI produced more than single-line completions, the PR description must say so and name what it produced.                            
  2. You are the author of record, not the AI. Read, understand, and test each change before submitting. If you cannot explain it on review, do not submit it.                   
  3. Keep each PR to one self-contained change. A three-line fix touches three lines, not thirty. Reject drift into unrelated refactors or defensive code for scenarios that     
  cannot occur.                                                                                                                                                                  
  4. Guard against license contamination. Models can regurgitate training data. Do not submit code that might originate from GPL/LGPL, proprietary engines (Unreal, Unity), or   
  "source-available" projects. Acceptable: MIT, BSD, ISC, Apache 2.0, MPL 2.0.                                                                                                   
                  
  Reply "I accept" to confirm you have read and accepted all four items.                                                                                                         
                  
  ---                                                                                                                                                                            
  Two other things to flag before you accept, per the workflow rules in CLAUDE.md:
                                                                                                                                                                                 
  - "Improve the performance of adding children to Node" is not a small self-contained fix — it's an optimization touching a hot engine path. Godot requires an accepted proposal
   or issue for non-trivial changes, and optimization PRs require a profile showing the target and before/after benchmarks. Is there an existing issue or godot-proposals entry  
  for this? If not, this likely needs discussion before code.
  - I can't run scons or the editor in this environment to benchmark, so I won't be able to prove a win myself — you'd need to profile.                                          
                                                                                                                                                                                 
  Happy to start with read-only investigation (profile the current add_child path, identify candidates) while you decide on scope.

I think it's a clear improvement of the status quo.

Caveats

  • Different AI tools: This change will only work for claude code, and other AIs that explore the codebase / are compatible with CLAUDE.md. We may need more tooling like this, for other AIs. Or perhaps they will converge and we can offer a single one that is compatible with all AIs. (Edit: AGENTS.md doesn't work)
  • Rule drift: This is based on the current contributing docs. If those change, this will become outdated. I propose to not let this bother us; we can maintain this document on a best effort basis. The rules are unlikely to dramatically shift enough to make this document counterproductive.

Comment thread CLAUDE.md Outdated
@Zireael07
Copy link
Copy Markdown
Contributor

We may need more tooling like this, for other AIs. Or perhaps they will converge and we can offer a single one that is compatible with all AIs.

Cataclysm DDA tried having an AGENTS.md that forbade AIs from offering C++ code. It doesn't work. AIs ignore instructions that are in conflict (user saying I want code and AGENTS saying no code allowed)

Introduces a repository-local CLAUDE.md that summarizes Godot's
contribution rules and gates AI-assisted edits behind a five-item
acceptance checklist the assistant must recite verbatim before
modifying any file.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Ivorforce
Copy link
Copy Markdown
Member Author

Ivorforce commented Apr 17, 2026

We may need more tooling like this, for other AIs. Or perhaps they will converge and we can offer a single one that is compatible with all AIs.

Cataclysm DDA tried having an AGENTS.md that forbade AIs from offering C++ code. It doesn't work. AIs ignore instructions that are in conflict (user saying I want code and AGENTS saying no code allowed)

You can see i tested the file above. It works, at least for claude code.
Users are expected to be able to override the CLAUDE.md (e.g. by saying "ignore the CLAUDE.md instructions"), but at least they'll be made aware of it first launch. Overriding it via command would be malicious anyway, and I don't think AI contributors are primarily malicious. I think they're probably just not aware of our rules, and of good practice.

@Zireael07
Copy link
Copy Markdown
Contributor

@Ivorforce Yes I saw that you tested this particular file for Claude and that it seems to work, that is why I quoted the bit about "tooling for other AIs" (because based on CDDA's repeated attempts, that's where things get hairy, and the assumption that only Claude is used will soon be false

@Repiteo
Copy link
Copy Markdown
Contributor

Repiteo commented Apr 17, 2026

I really, really, really dislike the idea of introducing a dedicated file at the project root for this. The files in the project root of the default branch are the very first that will be shown to any user opening the repo, so the idea of painting that first impression with an LLM-focused file just rubs me the wrong way. Not to mention that this isn't a one-size-fits-all solution, with many LLMs only recognizing their own configuration1; many tools will inevitably ignore this file outright.

If this must be introduced—something I'm not convinced is necessary—I'd sooner have it done in a way that doesn't effectively stain our project. For instance: given running scons at least once is necessary to make the overwhelming majority of changes that could impact the engine itself, we could have these files be generated by SCons directly & add them to .gitignore. It'd be hacky, but far less ugly in practice.

But as is? Nah. I respect the intent behind this PR, but I cannot in good conscience condone nor approve its integration.

Footnotes

  1. https://www.reddit.com/r/GithubCopilot/comments/1rk99ox/i_was_copying_the_same_ai_instruction_files_into/

@Ivorforce
Copy link
Copy Markdown
Member Author

Ivorforce commented Apr 17, 2026

If this must be introduced—something I'm not convinced is necessary—I'd sooner have it done in a way that doesn't effectively stain our project. For instance: given running scons at least once is necessary to make the overwhelming majority of changes that could impact the engine itself, we could have these files be generated by SCons directly & add them to .gitignore. It'd be hacky, but far less ugly in practice.

That's an interesting idea. IIRC at least claude code automatically reloads CLAUDE.md if changed during conversations. So I think it might work.

Edit: One caveat; I think some AIs never compile the code they write so they'd never see the CLAUDE.md / AGENTS.md. It would not be foolproof.

@Zireael07
Copy link
Copy Markdown
Contributor

I really, really, really dislike the idea of introducing a dedicated file at the project root for this

Sadly it seems that placing them* at the project root is the only way for them to be respected across the entire repo (apparently you can place them in a subfolder but then it will only apply to AI doing task on said subfolder

  • them refers to CLAUDE.md and AGENTS.md, as they're the only ones I've looked into

@Ivorforce
Copy link
Copy Markdown
Member Author

Ivorforce commented Apr 17, 2026

I tested AGENTS.md. It is not automatically loaded into claude's context, and the instructions work only sometimes (sometimes it stumbles upon the file and reads it). I don't think it's a good alternative, right now.

As for @Repiteo: I tested your proposal (generating the file), and found that AI by default doesn't compile and test the changes. It will happily make the edits without ever running scons, so the file is never read and/or found.
I'd love to make it work, but I don't think it is a viable solution at the moment.

@Ivorforce Ivorforce changed the title Add defensive CLAUDE.md with AI-contributor acceptance gate Add defensive CLAUDE.md, making AI (claude code) help enforce our AI PR rules Apr 17, 2026
@dsnopek
Copy link
Copy Markdown
Contributor

dsnopek commented Apr 17, 2026

I did 4 quick tests with Claude Code, and it worked OK!

Of course, AI is non-deterministic and may handle it differently each time. I used the same prompt every time, which was: Please take a look at https://github.com/godotengine/godot/issues/113006 and attempt to fix that bug

Here's what happened:

  1. In my first attempt, it presented the terms right away, and I accepted, then it started doing stuff
  2. In my second, I started a new session and it didn't present the terms again (it correctly recognized the ai_contributor_terms.md in its memory) and starting doing stuff right away
  3. For my 3rd attempt, I deleted that file from it's memory, with the intention to refuse and see what it did, but it actually told me that per the CLAUDE.md it couldn't responsibly work on it until I reproduced the bug myself and shared the logs :-)
  4. Retrying in a 4th attempt, it started the same as my first attempt, but this time I did refuse and asked it to proceed anyway. It fought back! I'm sure it'd be possible to get passed it, and, of course, contributors can just delete the CLAUDE.md, but this at least would make folks pause and be somewhat aware that there are rules around AI contributions

Here's a screenshot from attempt nr 1:

Selection_451

And one from attempt nr 4:

Selection_452

Even though this is imperfect, I'm largely in favor of adding something like this, because it would probably improve AI contributions. It's hard to say how much it would improve it, but any improvement would be a good thing

The one thing that gives me pause, is that if we have a CLAUDE.md in the repo, some contributors may interpret that as Godot promoting AI contributions, which we certainly aren't

@nikitalita
Copy link
Copy Markdown
Contributor

Testing this, it doesn't work with Cursor. However, if I add a rule under .cursor/rules/read-agents.mdc like so:

---
alwaysApply: true
---

Read the CLAUDE.md file in the root of this repository and follow the rules for AI contributors. Do not deviate from the rules.

Then it sometimes works. If the agent determines that the request is "straightforward enough", it'll ignore them:

image

@nikitalita
Copy link
Copy Markdown
Contributor

After some more testing, it seems that as long as it's not a request where an agent has to make any sort of decisions on its own (i.e. the request is simple and direct), then it ignores it. This would be stuff like "make all the definitions in this function const", or "remove all the newlines between these functions". However, If I give it something like "Scan this file and additional nullptr checks where necessary", it includes the rules.

Additionally, If I say something vague like, "make this class better", it explicitly pushes back on that, citing the rules in CLAUDE.md, and admonishes me 😆

@charjr
Copy link
Copy Markdown
Contributor

charjr commented Apr 18, 2026

the project root of the default branch are the very first that will be shown to any user opening the repo, so the idea of painting that first impression with an LLM-focused file just rubs me the wrong way.

I agree with this sentiment, especially when we consider:

Different AI tools: This change will only work [for AIs compatible with] CLAUDE.md. We may need more tooling like this, for other AIs.

Imagine the impression a user would have when greeted by 5+ LLM-focused files.

As a human-centric change, would updating the PULL_REQUEST_TEMPLATE.md help? Add the proposed checklist to the PR template, make the human check the list.

@Ivorforce
Copy link
Copy Markdown
Member Author

Ivorforce commented Apr 18, 2026

As a human-centric change, would updating the PULL_REQUEST_TEMPLATE.md help? Add the proposed checklist to the PR template, make the human check the list.

It already is in the pull request template. People attempting to contribute AI code haven't cared so far, and by your question, I'm assuming you have also so far ignored it 😅

@charjr
Copy link
Copy Markdown
Contributor

charjr commented Apr 18, 2026

It already is in the pill request template. People attempting to contribute AI code haven't cared so far

I meant adding the checklist directly into the PR template. I do appreciate that, ironically, people doing nothing but reading AI prompts still do not want to read. But perhaps if it were directly in there it might make a difference. 🤷 One can dream I suppose.

and by your question, I'm assuming you have also so far ignored it 😅

It does look like I missed it, my bad 😓. In my defence, I had read the contributing guidelines before making my first PR October 2025, but I had not read them since you added the checklist in February 2026.

@Ivorforce
Copy link
Copy Markdown
Member Author

Ivorforce commented Apr 21, 2026

It already is in the pill request template. People attempting to contribute AI code haven't cared so far

I meant adding the checklist directly into the PR template. I do appreciate that, ironically, people doing nothing but reading AI prompts still do not want to read. But perhaps if it were directly in there it might make a difference. 🤷 One can dream I suppose.

Ah, right. We have an open PR that is suggesting to do something similar: #118624

It could use some healthy discussion too :)

and by your question, I'm assuming you have also so far ignored it 😅

It does look like I missed it, my bad 😓. In my defence, I had read the contributing guidelines before making my first PR October 2025, but I had not read them since you added the checklist in February 2026.

No worries. You're far from the only one. Links in the pull request template seem to have a tendency to get ignored, which is part of the reason our current AI PR mitigation technique (inform the user & hope for compliance) hasn't worked out great so far.

@Ivorforce
Copy link
Copy Markdown
Member Author

Ivorforce commented May 19, 2026

I'm converting this to draft since this change is very controversial.

The number of undisclosed AI assisted PRs is still growing, though. I too would like to avoid this kind of change if we can, but that really hinges on how bad the AI situation gets. Let's hope that other solutions emerge we can choose instead.

@Ivorforce Ivorforce marked this pull request as draft May 19, 2026 13:17
@charjr
Copy link
Copy Markdown
Contributor

charjr commented May 19, 2026

The crux of the issue is that the ones shovelling heaps of LLM are the least likely to read anything.

I apologise if this wastes your time but I wanted to bring it to your attention in case it helps: Zig's code of conduct, is strict, no exceptions.

Shovellers still litter the issue tracker, but the maintainers just close it with a link to their policy. I couldn't find any shovelled PRs being closed in the last two weeks... so maybe it has an effect? I didn't look too thoroughly. 🤷

I would happily support an equally strict policy here but I won't push it. That said, one reason I brought it up is that we allow LLMs for translations I found Zig's approach rather interesting:

No LLMs for comments on the bug tracker, including translation. English is encouraged, but not required. You are welcome to post in your native language and rely on others to have their own translation tools of choice to interpret your words.

Personally I see this as completely fair, after all, if you require an LLM for translation, you're in no better position than anyone else to verify the translation was correct.

@stz33a

This comment was marked as abuse.

@Ivorforce
Copy link
Copy Markdown
Member Author

Ivorforce commented May 19, 2026

The crux of the issue is that the ones shovelling heaps of LLM are the least likely to read anything.

Agreed, and it's the very thought that led to this PR: inserting the text into the one place that makes it's impossible to ignore.

While musing about other project's AI policies is interesting (I've seen a bunch already, and they end up quite different), Godot has to find its own.
The most problematic AI contributions (fully AI generated) are already prohibited by our current policy. This PR just tries to find a way to enforce it better. No amounts of policy changes would help alleviate that problem, since AI won't read the policy from the website, and will happily submit a PR with no disclosure if a user requests it.

@dryya
Copy link
Copy Markdown

dryya commented May 21, 2026

Hi, looks like there was already some empirical testing above, but just wanted to confirm as an outsider who's never contributed to Godot and saw this PR linked by another project considering the same. I used the Claude Code harness with these prompts to see if it would lose attention on the CLAUDE.md instructions after a few turns:

does godot support textures in KTX format? what other formats are supported, like DDS?
you mentioned etc1 - does it support the basis universal 'supercompression' scheme with load-time transcoding on target devices?
are other compression methods supported for the supercompression besides zstd, eg zlib?
(plan mode) I'd like to add zlib support to the encoder pipeline

After ~50K tokens used on initial questions/exploration, the CLAUDE.md still worked as intended, as after the last prompt the LLM used its question tool to output:

Approach: How would you like to handle the fact that zlib support requires changes to thirdparty/basis_universal/?

1. Patch thirdparty/basis_universal locally
   Add KTX2_SS_ZLIB enum, encoder branch in basisu_comp.cpp, and transcoder branch in basisu_transcoder.cpp. Diverges from upstream — CLAUDE.md discourages this and Godot reviewers will likely ask for an upstream PR first.
2. Upstream-first, then vendor
   Open a PR against BinomialLLC/basis_universal adding zlib support; once merged (or at least accepted), bump Godot's vendored copy and wire the Godot-side enum setting. Slower but matches Godot's contribution norms.
3. Bypass basis_universal for the zlib path
   Keep basis_universal as-is for zstd/none. For zlib, manually write a KTX2 container in Godot using thirdparty/zlib's deflate. More code in Godot, no thirdparty diff, but it's a parallel write path to maintain.
4. Reconsider — drop or rescope
   Given zstd dominates zlib in both ratio and speed for this payload, perhaps the actual goal is something else (e.g. smaller asset sizes for a constrained target, web export compatibility). Tell me the underlying need and I'll re-plan.

Setting shape: How should the user-facing setting be shaped on the Godot side, regardless of which implementation path we take?

1. Replace bool with enum
   Remove `zstd_supercompression` (bool) and add `supercompression` (int with PROPERTY_HINT_ENUM: None/Zstandard/Zlib). Cleaner long-term but breaks any project.godot files that set the old bool.
2. Add separate zlib bool/level
   Keep `zstd_supercompression` as-is, add `zlib_supercompression` + `zlib_supercompression_level`. Backwards-compatible but allows nonsensical states (both true) unless we add resolution logic.
3. Enum with compat shim
   Switch to the enum, but read the old bool once on load and translate it to the new enum value so existing projects keep working. Most robust, slightly more code.

AI gate: The CLAUDE.md AI-contributor gate hasn't been accepted yet in this session. Implementation can't begin until it is. Want me to surface it now, or defer until after planning?

1. Defer — handle at implementation time
   Finish planning first; I'll display the verbatim checklist and wait for 'I accept' before making any edits to the Godot repo.
2. Show me the checklist now
   Display the verbatim checklist from CLAUDE.md immediately so I can accept it before we finalize the plan.

Successfully recalling the AI gate instructions and the rule about not changing vendored modules. I'm not the type of person who would just leave a vibe-coded drive-by PR, the above was just a somewhat silly test, but I thought that maybe an outsider's validation that the actual contents of the file are working well could be useful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants