-
-
Notifications
You must be signed in to change notification settings - Fork 970
Add AGENTS.md and AI coding assistant support #15340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.0.x
Are you sure you want to change the base?
Add AGENTS.md and AI coding assistant support #15340
Conversation
jdaugherty
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of information missing from this file. Some of it:
- We need to inform LLMs of our project structure (it's not that it's multiproject, but the composite build is important to mention). This includes directory discussions.
- Build instructions need run - rat & codestyle need explicitly mentioned
- We need rules on restructuring the project
- We need rules on dependency includes and when to include, where
The purpose of this file is to make it easier for the LLM to understand how to run tests, how to make changes, etc. While this is a good starting point, we need to be much more thorough.
|
AGENTS.md - some items we might want to include |
|
@sanjana2505006 I am going to push some additional changes to this PR based on https://github.com/apache/superset/ and the conversation in our weekly Grails planning meeting. |
Replaces the previous ASF license header notes with a comprehensive guide for automated coding agents and developers. Adds detailed instructions on environment setup, build/test commands, code style, Spock/Groovy conventions, commit/PR guidelines, parallel test isolation, and documentation processes for the grails-core repository.
Add three specialized skills in .agents/skills/ for AI agent guidance: - grails-developer: Grails 7 development with GORM, controllers, services, views, plugins, and Spock/Geb testing - groovy-developer: Groovy 4 syntax, closures, DSLs, metaprogramming, static compilation, and testing patterns - java-developer: Java 17 features including records, sealed classes, pattern matching, and Groovy interoperability Skills follow the open AGENTS.md standard format with compatibility for Claude, Copilot, Cursor, Gemini, Windsurf, Cline, and others.
Add instruction files that reference AGENTS.md for various AI tools: - CLAUDE.md: Claude Code - GEMINI.md: Google Gemini CLI - .github/copilot-instructions.md: GitHub Copilot - .cursorrules: Cursor - .windsurfrules: Windsurf - .clinerules: Cline Update AGENTS.md with optimized agent-friendly format: - Quick reference section with common commands - Critical rules highlighted (jakarta.*, @GrailsCompileStatic, etc.) - Concise tables for technology stack, modules, and artefacts - Code patterns for Spock tests, mocking, and framework access - Test isolation warnings for parallel execution - Troubleshooting table for common issues All files include Apache License headers and reference the skills in .agents/skills/ for detailed guidance.
|
We will likely want to copy this over to grails-spring-security, grails-cache and grails-quartz. |
Eliminates duplication by making all AI assistant config files symlinks to the canonical AGENTS.md, ensuring content loads automatically.
Updated AGENTS.md to require AI agents to read relevant skill files before writing or modifying Groovy, Grails, or Java code. This ensures agents follow best practices and guidelines specific to each language or framework.
Creates .claude/skills/ directory with symlinks to the canonical skill files in .agents/skills/, enabling Claude Code slash commands.
jdaugherty
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good start
| ./gradlew bootRun | ||
|
|
||
| # Run tests | ||
| ./gradlew test # All tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not make this ./gradlew check instead? I know that unit & integration tests both extend the test type, but check is meant to run everything I thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have a strong opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe check will run all verification tasks like codenarc* and checkstyle*?
|
@sanjana2505006 Let's let @jamesfredley implement the feedback in this PR and we'll merge once @matrei also reviews. |
Deleted the .github/copilot-instructions.md symlink that pointed to ../AGENTS.md. This cleans up unused or obsolete configuration.
Added a new section detailing the repository's multi-project Gradle structure and build commands. Introduced pull request guidelines and review process table. Updated security reporting instructions to reference the ASF Security Team.
matrei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 This is a great.
|
|
||
| ## Test Isolation | ||
|
|
||
| > **WARNING**: Tests run in parallel (`maxParallelForks > 1`). Static state causes flaky tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static state does not cause flaky tests in parallel forks. It is when neglecting to reset the environment and any changed static state in test cleanup, that the next sequential tests (in the same fork) can be affected.
|
|
||
| - Use `GrailsWebRequest.lookup()` for thread-local context | ||
| - Clear artefacts: `grailsApplication.artefactInfo.clear()` | ||
| - Use `@ResourceLock` for shared resources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ResourceLock is used with Spock parallel execution. We do not use Spock parallel execution. @Shared is used for fields that should be reused by multiple feature methods in a Spec.
| books*.title | ||
|
|
||
| // DO: Static compilation | ||
| @GrailsCompileStatic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or @CompileStatic if this is not a Grails artefact or not using any Grails specific dynamic conventions
| |---------|----------| | ||
| | Out of memory | `export GRADLE_OPTS="-Xms2G -Xmx5G"` | | ||
| | Container missing | Use `-PskipTests` or install Docker/Podman | | ||
| | Flaky tests | Check static state pollution, use `@ResourceLock` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove @ResourceLock?
| - **Grails 7 Guide**: https://docs.grails.org/latest/guide/single.html | ||
| - **Groovy 4 Docs**: https://docs.groovy-lang.org/docs/groovy-4.0.30/html/documentation/ | ||
| - **Spock 2.3 Docs**: https://spockframework.org/spock/docs/2.3/all_in_one.html | ||
| - **GORM Docs**: https://gorm.grails.org/latest/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Prefer Elvis operator (`?:`) for default values. | ||
| - Use GStrings for string interpolation. | ||
| - Prefer traits over inheritance for mixins. | ||
| - Use explicit types for public APIs, `def` for local variables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐
| - Use explicit types for public APIs, `def` for local variables. | ||
|
|
||
| ### Grails-Specific | ||
| - Use `@GrailsCompileStatic` instead of `@CompileStatic` in Grails classes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...if needed
| ## Common Pitfalls | ||
|
|
||
| - **`==` vs `is`**: `==` calls `equals()`, use `is` for identity comparison. | ||
| - **GString in maps**: Use `"key".toString()` as map keys if needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example should use GString, for example "key$idx".toString()
| - **`==` vs `is`**: `==` calls `equals()`, use `is` for identity comparison. | ||
| - **GString in maps**: Use `"key".toString()` as map keys if needed. | ||
| - **Closure scope**: Understand `delegate`, `owner`, and `this` in closures. | ||
| - **Optional parentheses**: Can cause ambiguity; use parentheses when unclear. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐ (I would say, always use parentheses, much clearer and easier to read)
| - **GString in maps**: Use `"key".toString()` as map keys if needed. | ||
| - **Closure scope**: Understand `delegate`, `owner`, and `this` in closures. | ||
| - **Optional parentheses**: Can cause ambiguity; use parentheses when unclear. | ||
| - **Static imports**: Groovy may not find statically imported methods without explicit import. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this mean? Does this differ from Java?
Summary
Add comprehensive support for AI coding assistants working with the Grails codebase:
.agents/skills/for Grails 7, Groovy 4, and Java 17.claude/skills/enabling slash commands for developer skillsArchitecture
All agent instruction files are symlinks to AGENTS.md, ensuring:
Files Added
AGENTS.md.agents/skills/grails-developer/SKILL.md.agents/skills/groovy-developer/SKILL.md.agents/skills/java-developer/SKILL.mdCLAUDE.mdGEMINI.md.github/copilot-instructions.md.cursorrules.windsurfrules.clinerules.claude/skills/*/SKILL.mdAGENTS.md Highlights
Skills Coverage
Test Plan
ls -laandcat)