From 47b9c375083272216560380c0cd94fb191081cd4 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 17:57:26 -0500 Subject: [PATCH 01/20] Remove dangling codebase-memory skill reference from AGENTS.md The mandatory skill-reading list pointed agents at .agents/skills/codebase-memory/SKILL.md, which was never created and isn't listed in the skills table. Drop the reference. --- AGENTS.md | 1 - 1 file changed, 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 5ec9ac09512..8f41e4aa9f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,7 +66,6 @@ export GRADLE_OPTS="-Xms2G -Xmx5G" > - Writing Hibernate code → Read `.agents/skills/hibernate-developer/SKILL.md` > - Fixing style/analysis violations → Read `.agents/skills/violation-fixer/SKILL.md` > - Fixing broken test → Read `.agents/skills/test-fixer/SKILL.md` -> - Indexing code -> Read `.agents/skills/codebase-memory/SKILL.md` > > Use your file reading capability to load the skill content before proceeding with any code changes. From eb8294d9b6d4d57d3040f5982281ccf426679633 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 18:01:41 -0500 Subject: [PATCH 02/20] Add adversarial self-review step to PR guidelines Agents should run an adversarial review pass (e.g. /code-review or a fresh-context agent) against AGENTS.md's rules and the change's own logic before requesting human review, so scarce reviewer time goes to judgment calls rather than mechanical rule violations. Supplements, does not replace, the human review gate. Also fixes the pre-existing duplicate "6." numbering in this list. --- AGENTS.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8f41e4aa9f8..e4da06e3c23 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -252,8 +252,9 @@ class MyService { } 3. **Run code style checks**: `./gradlew codeStyle` 4. **Clean violations**: Before committing, run `./gradlew clean aggregateViolations` from the root and ensure that `build/reports/violations/CHECKSTYLE_VIOLATIONS.md`, `build/reports/violations/CODENARC_VIOLATIONS.md`, `build/reports/violations/PMD_VIOLATIONS.md`, and `build/reports/violations/SPOTBUGS_VIOLATIONS.md` have no issues. 5. **Verify test coverage**: Ensure any touched class is covered by tests verifying all behavior. You must run ALL tests in the affected module(s) and ensure they pass before submission. -6. **Squash commits** into a single meaningful commit message -6. **Reference issues** in PR description (e.g., "Fixes #1234") +6. **Adversarial self-review**: Before requesting human review, run an adversarial review pass (e.g. `/code-review`, or a fresh-context agent) against this file's rules — jakarta not javax, no wildcard imports, BOM version rules, test coverage — and against the change's own logic. Fix or flag anything it finds. This is a supplement to human review, not a replacement for it. +7. **Squash commits** into a single meaningful commit message +8. **Reference issues** in PR description (e.g., "Fixes #1234") ### Review Process From d9f112828bf24b13e5f5b5a06d2dba8eeee74181 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 18:41:31 -0500 Subject: [PATCH 03/20] Default to single large PRs over reviewability stacks PR chains (e.g. the Neo4j GormRegistry migration: #15779-#15817, consolidated into #15972) were used to keep diffs reviewable, but GitHub's Copilot reviewer silently skips PRs over ~300 files - #15972's consolidated diff got no automated review, and the integration between its sub-PRs was never itself reviewed. Since adversarial self-review is now mandatory on every PR regardless of size, splitting for reviewability no longer buys coverage, only coordination overhead. Default to one PR per feature/migration; split only for independently revertable/mergeable units. --- AGENTS.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index e4da06e3c23..1abe2dab57d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -247,12 +247,31 @@ class MyService { } ## Pull Request Guidelines +### PR Sizing: Prefer One Large PR Over a Reviewability Stack + +Stacking a feature/migration into a chain of small PRs (as with the Neo4j GormRegistry +migration: #15779 → #15780 → #15790 → #15816 → #15817, later consolidated into #15972) +was a strategy for keeping individual diffs small enough for human reviewers and for +GitHub's Copilot PR reviewer, which refuses to review a PR over ~300 files (#15972 hit +this cap and got zero automated review on the consolidated result — each sub-PR had been +reviewed individually, but the integration between them never was). + +Now that adversarial self-review (Guideline 6 below) is mandatory on every PR regardless +of size, and collaborators are routinely using agents to review PRs, splitting for +reviewability no longer buys review coverage — it only adds coordination overhead +(rebasing a stack, keeping sub-PRs in sync, re-reviewing everything again at consolidation +time). **Default to a single PR for a feature or migration.** Split only for reasons other +than reviewability — e.g. independently revertable or independently mergeable units of +work. + +### Checklist + 1. **Fork & branch** from the target release branch (e.g., `7.0.x`) 2. **Run tests** before submitting: `./gradlew build --rerun-tasks` 3. **Run code style checks**: `./gradlew codeStyle` 4. **Clean violations**: Before committing, run `./gradlew clean aggregateViolations` from the root and ensure that `build/reports/violations/CHECKSTYLE_VIOLATIONS.md`, `build/reports/violations/CODENARC_VIOLATIONS.md`, `build/reports/violations/PMD_VIOLATIONS.md`, and `build/reports/violations/SPOTBUGS_VIOLATIONS.md` have no issues. 5. **Verify test coverage**: Ensure any touched class is covered by tests verifying all behavior. You must run ALL tests in the affected module(s) and ensure they pass before submission. -6. **Adversarial self-review**: Before requesting human review, run an adversarial review pass (e.g. `/code-review`, or a fresh-context agent) against this file's rules — jakarta not javax, no wildcard imports, BOM version rules, test coverage — and against the change's own logic. Fix or flag anything it finds. This is a supplement to human review, not a replacement for it. +6. **Adversarial self-review (always, regardless of PR size)**: Run an adversarial review pass (e.g. `/code-review`, or a fresh-context agent) against this file's rules — jakarta not javax, no wildcard imports, BOM version rules, test coverage — and against the change's own logic. Do this for every PR, including large/consolidated ones; do not rely on GitHub's Copilot reviewer alone, since it silently skips PRs over ~300 files. Fix or flag anything it finds. This is a supplement to human review, not a replacement for it. 7. **Squash commits** into a single meaningful commit message 8. **Reference issues** in PR description (e.g., "Fixes #1234") From 7d6259acd4adf31ad4e1b6d9a3d2df5ad1660226 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 18:48:06 -0500 Subject: [PATCH 04/20] Replace hardcoded skill index with directory-based discovery The task->skill bullet list and the skills table both had to be kept in sync with .agents/skills/ by hand, and had already drifted twice: a reference to codebase-memory/SKILL.md that never existed, and mono-repo-integration/SKILL.md existing on disk but referenced nowhere in this file. Point agents at the directory instead: enumerate .agents/skills/*/ and match on each SKILL.md's own front-matter description. Adding, renaming, or removing a skill no longer requires touching AGENTS.md, and this works for any agent that reads front-matter, not just ones that follow a Claude-specific list. --- AGENTS.md | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1abe2dab57d..a9af25dc8c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,26 +58,13 @@ export GRADLE_OPTS="-Xms2G -Xmx5G" ## Available Skills -> **AI AGENTS - MANDATORY**: Before writing or modifying any code, you **MUST** read the relevant skill file(s) below. Do not write Groovy/Grails code without first loading these instructions: -> - Writing Grails code → Read `.agents/skills/grails-developer/SKILL.md` -> - Writing Groovy code → Read `.agents/skills/groovy-developer/SKILL.md` -> - Writing Java code → Read `.agents/skills/java-developer/SKILL.md` -> - Upgrading applications to Grails 8 → Read `.agents/skills/grails-8-upgrade/SKILL.md` -> - Writing Hibernate code → Read `.agents/skills/hibernate-developer/SKILL.md` -> - Fixing style/analysis violations → Read `.agents/skills/violation-fixer/SKILL.md` -> - Fixing broken test → Read `.agents/skills/test-fixer/SKILL.md` +> **AI AGENTS - MANDATORY**: Before writing or modifying any code, list `.agents/skills/*/SKILL.md`, read each one's front-matter `description`, and load the full file for any skill whose description matches the task at hand. Do not write Groovy/Grails/Java code without first loading the skill(s) that apply. > -> Use your file reading capability to load the skill content before proceeding with any code changes. - -| Skill | Path | Use For | -|-------|------|---------| -| **grails-developer** | `.agents/skills/grails-developer/SKILL.md` | Current Grails apps, GORM, controllers, views | -| **groovy-developer** | `.agents/skills/groovy-developer/SKILL.md` | Groovy 5 syntax, closures, DSLs, Spock | -| **grails-8-upgrade** | `.agents/skills/grails-8-upgrade/SKILL.md` | Upgrading Grails applications from 7.x to 8 | -| **java-developer** | `.agents/skills/java-developer/SKILL.md` | Java 21 features, Groovy interop | -| **hibernate-developer** | `.agents/skills/hibernate-developer/SKILL.md` | Hibernate 7 mapping, binders, generators | -| **violation-fixer** | `.agents/skills/violation-fixer/SKILL.md` | Fix style/analysis violations (CodeNarc, Checkstyle, PMD, SpotBugs) | -| **test-fixer** | `.agents/skills/test-fixer/SKILL.md` | Aggregate and fix test failures | +> ```bash +> for f in .agents/skills/*/SKILL.md; do awk -F': *' '/^description:/{print FILENAME": "$2; exit}' "$f"; done +> ``` +> +> The directory is the source of truth, not a list in this file — a hardcoded skill index here would drift the moment a skill is added, renamed, or removed. Each `SKILL.md`'s front-matter (`name`, `description`, `compatibility`) is what makes it discoverable to any agent, per the Agent Skills Specification. ## Technology Stack From 1cf068fbba6ffba6ac556c121ad0a69e79a0c938 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 18:58:28 -0500 Subject: [PATCH 05/20] Add worktree-hygiene skill for .claude/worktrees/ sprawl .claude/worktrees/ accumulates agent-created worktrees with no documented cleanup process - found 3 stale ones from a 2026-06-25 workflow run still present today, plus a since-removed one for a branch whose PR had long since merged. Age/idle time is the wrong staleness signal here: this repo's PR chains go quiet between review rounds without being abandoned, so a worktree is valid as long as its branch has a live remote or open PR. The skill classifies each worktree under .claude/worktrees/ by GitHub PR state and merge-into-default-branch status instead, checks for uncommitted work before recommending anything, and never touches worktrees outside .claude/worktrees/ or removes anything without explicit confirmation. No AGENTS.md edit needed to make this discoverable - it surfaces through the directory-based skill discovery added in the previous commit. --- .agents/skills/worktree-hygiene/SKILL.md | 98 ++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 .agents/skills/worktree-hygiene/SKILL.md diff --git a/.agents/skills/worktree-hygiene/SKILL.md b/.agents/skills/worktree-hygiene/SKILL.md new file mode 100644 index 00000000000..6b39f923155 --- /dev/null +++ b/.agents/skills/worktree-hygiene/SKILL.md @@ -0,0 +1,98 @@ + +--- +name: worktree-hygiene +description: Reports stale or orphaned git worktrees under .claude/worktrees/ (the agent-managed worktree directory) by checking each worktree's branch against its GitHub PR state and merge-into-default-branch status, not commit age. Use at the start of a session in this repo when .claude/worktrees/ has accumulated entries, or when asked to clean up worktrees or check branch hygiene. Report-only — never deletes without explicit confirmation, and never touches worktrees outside .claude/worktrees/. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +metadata: + audience: maintainers + frameworks: grails +--- + +## What I Do + +- Enumerate worktrees under `.claude/worktrees/` — the agent-managed worktree directory. Never touch worktrees elsewhere (e.g. a contributor's own manually created `../grails-core-8.0.x`); those aren't mine to judge. +- Classify each one's branch as merged, closed-unmerged, still active, or orphaned, using GitHub PR state as the authoritative signal — not commit age or idle time. +- Report findings as a table. Never run `git worktree remove` or `git branch -D` without explicit confirmation, especially where the worktree has uncommitted changes. + +## When to Use Me + +- At the start of a session in this repo, if `.claude/worktrees/` has more than a couple of entries. +- When asked to "clean up worktrees" or "check branch hygiene." +- Before creating a new worktree, to check whether an existing one for related work should be reused instead of piling on another. + +--- + +## Why Age/Idle-Time Is the Wrong Signal + +A worktree can sit untouched for weeks and still be exactly where it should be. This repo's PR chains (e.g. `feat/gorm-datastore-infra` → `feat/gorm-registry-core-impl` → `test/gorm-registry-core-tests` → ... → `feat/neo4j-gorm-registry-migration`) go quiet between review rounds without being abandoned — a worktree is valid for as long as its branch has a live remote or an open PR, no matter how stale it looks. The only signal that reliably distinguishes "waiting on review" from "actually dead" is PR/remote state, not the timestamp of the last commit. + +## Classification Procedure + +List the worktrees in scope: + +```bash +git worktree list | grep '\.claude/worktrees/' +``` + +For each `` found: + +### 1. Remote tracking status + +```bash +git branch -vv | grep -F "$branch " +``` + +Look for `: gone]` — the remote branch was deleted, usually after a merge or a PR close. + +### 2. PR state (authoritative — catches squash/rebase merges, where commit-ancestry checks alone give a false negative) + +```bash +gh pr list --repo apache/grails-core --head "$branch" --state all \ + --json state,number,title,mergedAt,url +``` + +- `state: MERGED` → safe-to-remove candidate. +- `state: OPEN` → active. **Leave alone.** A live PR means the worktree is doing its job regardless of idle time. +- `state: CLOSED` (not merged) → abandoned or superseded. Flag for confirmation before removing — closure doesn't always mean the work was worthless. +- No PR found → likely local-only or workflow-generated (e.g. a `worktree-wf_*` branch left over from an isolated workflow run). Fall through to step 3. + +### 3. Merge-into-default-branch check (only for branches with no PR record) + +```bash +default_branch=$(git symbolic-ref refs/remotes/origin/HEAD --short | sed 's@^origin/@@') +git merge-base --is-ancestor "$branch" "origin/$default_branch" && echo merged || echo not-merged +``` + +- Merged into the default branch → safe-to-remove candidate. +- Not merged and never pushed to any remote → orphaned workflow artifact. Flag for confirmation; check for uncommitted work first (step 4). + +### 4. Uncommitted work check (always run before recommending removal) + +```bash +git -C .claude/worktrees/ status -sb +``` + +Any modified or untracked files mean the worktree cannot be silently removed. Surface exactly what's uncommitted and let the user decide whether to commit, stash, or discard it before `git worktree remove` runs. + +## Output + +Report a table: worktree path, branch, classification (`MERGED` / `CLOSED-UNMERGED` / `OPEN-ACTIVE` / `ORPHANED`), uncommitted-changes flag, and a recommended action. Recommend the removal — do not execute it — and wait for confirmation. + +## Worked Example + +`.claude/worktrees/wf_57549255-d5f-5`, `-7`, and `-8` (found 2026-07-11): all three based on commit `d921eda90a`, which is a merged ancestor of the default branch (step 3 → `merged`), none had a corresponding open PR (step 2 → no PR found), and each had small uncommitted leftovers — an untracked `grails-data-hibernate7/` directory in two of them, a modified `GrailsDataTckManager.groovy` in the third (step 4). Correct classification: `ORPHANED`, flagged for confirmation rather than auto-removed, because of the uncommitted content. From 48d85126a6576c4a1aa954becdc48ecb238e1a69 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 19:12:48 -0500 Subject: [PATCH 06/20] Add micronaut-developer skill for grails-forge grails-forge is a Micronaut application (project generator behind start.grails.org), not a Grails app, but nothing in .agents/skills/ covered its actual idioms - DI, HTTP controllers, MicronautTest+Spock, Picocli CLI, Rocker templating, or the Feature extension-point system (82 files use @Singleton, 32 @Introspected, 20 @Inject, 9 @Controller, 8 @MicronautTest; zero skill coverage for any of it before this). Root AGENTS.md is also actively wrong for this subproject in at least one place: its "@GrailsCompileStatic, not @CompileStatic" rule is inverted here (0 @GrailsCompileStatic usages, 8 correct @CompileStatic ones), since grails-forge has no Grails artefacts. This skill documents where root rules do and don't transfer, verified against actual source rather than assumed. Grounds the earlier context-management finding: an agent working in grails-forge paid the full cost of root AGENTS.md's GORM/Hibernate/ artefact content with no corresponding Micronaut guidance to fill the actual gap. This is the prerequisite for a later scoped grails-forge/AGENTS.md. --- .agents/skills/micronaut-developer/SKILL.md | 191 ++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 .agents/skills/micronaut-developer/SKILL.md diff --git a/.agents/skills/micronaut-developer/SKILL.md b/.agents/skills/micronaut-developer/SKILL.md new file mode 100644 index 00000000000..31f0a9b915d --- /dev/null +++ b/.agents/skills/micronaut-developer/SKILL.md @@ -0,0 +1,191 @@ + +--- +name: micronaut-developer +description: Guide for working in grails-forge (grails-forge-core, grails-forge-api, grails-forge-cli, grails-forge-web-netty) — a Micronaut application, not a Grails one. Covers Micronaut DI/bean patterns, HTTP controllers, MicronautTest+Spock, Picocli CLI commands, Rocker templating, and the Feature extension-point system. Use this instead of grails-developer/hibernate-developer when changing code under grails-forge/. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +metadata: + audience: maintainers + frameworks: micronaut +--- + +## What I Do + +- Provide repository-specific guidance for `grails-forge/` — the project generator behind start.grails.org (the Grails equivalent of Spring Initializr). +- Cover Micronaut idioms actually used in this codebase: DI (`@Singleton`, `@Inject`), HTTP (`@Controller`, `@Get`, `@Post`), testing (`@MicronautTest` + Spock), and bean indexing for plugin-style extension points (`@Indexed`). +- Cover the Picocli CLI layer in `grails-forge-cli` and the Rocker templating engine used for both code generation output and API responses. +- Correct root `AGENTS.md` rules that do not apply here — see "Where Root AGENTS.md Rules Don't Apply" below. + +## When to Use Me + +Activate this skill instead of `grails-developer`/`hibernate-developer` when working on: + +- Anything under `grails-forge/**`. +- A new or modified `Feature` implementation (the generator's extension-point system). +- HTTP endpoints in `grails-forge-api`. +- CLI commands in `grails-forge-cli`. +- Rocker templates (`.rocker.raw`, `.rocker.html`) used for generated-project scaffolding or API rendering. + +## Module Context + +`grails-forge` is a **Micronaut application that generates Grails applications** — it is not itself a Grails app, and none of the GORM/artefact-handler/Hibernate content in root `AGENTS.md` applies to it. It's a multi-module Gradle build (own `settings.gradle`) with these modules: + +| Subproject | Role | +|---|---| +| `grails-forge-core` | Generation logic: the `Feature` system, templating, dependency/config assembly | +| `grails-forge-api` | HTTP API (Micronaut `@Controller`s) serving generation requests — backs start.grails.org | +| `grails-forge-web-netty` | Micronaut/Netty deployment of the API, shipped to Google Cloud Run | +| `grails-forge-cli` | Picocli command-line client hitting the same generation logic | +| `grails-forge-analytics-postgres` | Separate Postgres-backed analytics service (its own `Application`, controllers, repositories) | +| `test-core` | End-to-end specs that actually generate a project and verify it builds (e.g. `CreateAppSpec`, `CreateRestApiSpec`) | +| `grails-cli`, `grails-cli-shadow` | Legacy shell-packaging plumbing (shadow-jar distribution config) — not part of the generator logic, no Micronaut patterns here | + +There's also a separate React UI in `apache/grails-forge-ui` (a different repo) that consumes the API — not part of this codebase. + +## Key Patterns + +### Dependency Injection + +Standard Micronaut DI, constructor or field injection, `jakarta.inject.*` (not `javax.inject.*` — the one root `AGENTS.md` rule that *does* transfer here). + +```java +@Singleton +public class MongoSync extends MongoFeature { + public MongoSync(TestContainers testContainers) { + super(testContainers); + } + ... +} +``` + +### The Feature System (core domain concept) + +Every installable option in a generated project — a database driver, a test framework, a cloud integration — is a `@Singleton` implementing `org.grails.forge.feature.Feature` (or a category base class like `MongoFeature`), auto-discovered via Micronaut's compile-time `@Indexed(Feature.class)` bean indexing (no classpath scanning, no manual registry to update): + +```java +public interface Feature extends Named, Ordered, Described { + @NonNull String getName(); // unique feature id + default boolean isPreview() { return false; } + default boolean isCommunity() { return false; } + // ... getTitle(), getDescription(), apply(GeneratorContext), etc. +} +``` + +`apply(GeneratorContext)` is where a feature mutates the generated project's config and dependency list: + +```java +@Override +public void apply(GeneratorContext generatorContext) { + Map config = generatorContext.getConfiguration(); + config.put("grails.mongodb.url", "mongodb://${MONGO_HOST:localhost}:${MONGO_PORT:27017}/foo"); + generatorContext.addDependency(Dependency.builder() + .groupId("org.mongodb") + .artifactId("mongodb-driver-sync") + .implementation()); +} +``` + +New features go in `grails-forge-core/src/main/java/org/grails/forge/feature//`, grouped by `Category`. + +### HTTP Controllers + +Standard Micronaut HTTP, in `grails-forge-api`: + +```java +@Controller +public class ApplicationController { + @Get("/versions") + @Produces(MediaType.APPLICATION_JSON) + public HttpResponse versions(...) { ... } +} +``` + +### Testing: MicronautTest + Spock + +Not `HibernateGormDatastoreSpec` or any grails-core testing-support class — full HTTP round-trip tests via an injected client: + +```groovy +@MicronautTest +class ApplicationControllerSpec extends Specification { + @Inject + @Client("/") + HttpClient client + + void "test versions"() { + given: + def response = client.toBlocking().retrieve(HttpRequest.GET('/versions'), Map) + expect: + response.containsKey("versions") + } +} +``` + +### CLI Commands (Picocli, DI-wired) + +`grails-forge-cli` commands are Picocli `@CommandLine.Command` classes, DI-constructed: + +```java +@CommandLine.Command(name = CreateServiceCommand.NAME, description = "Creates a Service Class") +public class CreateServiceCommand extends CodeGenCommand { + public static final String NAME = "create-service"; + + @CommandLine.Parameters(paramLabel = "SERVICE-NAME", description = "...") + String serviceName; + + @Inject + public CreateServiceCommand(@Parameter CodeGenConfig config) { + super(config); + } + ... +} +``` + +### Templating: Rocker + +Code generation output (and some API rendering) uses Rocker templates (`.rocker.raw` / `.rocker.html`), compiled to Java classes and rendered via `RockerTemplate`/`RockerWritable`. Templates live alongside the feature or command that uses them (e.g. `feature/build/gitignore.rocker.raw`, `template/api/grailsForgeApi.rocker.raw`). + +## Where Root AGENTS.md Rules Don't Apply + +Verified against actual `grails-forge` source before writing this — don't take these on faith either, re-check if the codebase changes: + +- **Rule "Use `@GrailsCompileStatic`, not `@CompileStatic`" does not apply and is actively wrong here.** `grails-forge` has zero Grails artefacts and zero `@GrailsCompileStatic` usages; it correctly uses plain `@CompileStatic` (8 files). Don't "fix" this. +- **`GrailsWebRequest.lookup()`, GORM, artefact handlers**: none of these concepts exist in this codebase (0 occurrences). Ignore the root file's Artefact Types table, Key Modules table, and Test Isolation section when working here. +- **Code style tooling differs**: `grails-forge` uses Spotless (`spotlessJavaMisc`) and `checkstyleNohttp`, not the root project's CodeNarc/PMD/SpotBugs/`aggregateViolations` stack. Don't run `./gradlew clean aggregateViolations` expecting it to cover this subproject — it doesn't. Run `grails-forge`'s own style tasks from within `grails-forge/`. +- **Rule "jakarta.* not javax.*" does apply** — `grails-forge` follows it (108 files use `jakarta.*`), and Micronaut itself is jakarta-based, so this is one root rule that transfers cleanly. +- **Dependency/BOM management differs**: `grails-forge` has its own `settings.gradle`/`build.gradle` and its own version properties (`micronautVersion`, `picocliVersion`, etc.), independent of `dependencies.gradle`/`grails-bom`. The root `validateDependencyVersions` BOM rules don't govern this subproject. + +## Build & Test + +Run from inside `grails-forge/`, not the repo root: + +```bash +cd grails-forge +./gradlew build +./gradlew :grails-forge-api:test +./gradlew :grails-forge-cli:test +``` + +## Pitfalls to Avoid + +- Do not apply GORM/Hibernate/artefact-handler mental models here — this is a Micronaut HTTP service and CLI, not a Grails application. +- Do not add a new `Feature` without checking `@Indexed(Feature.class)` picks it up automatically via `@Singleton` — no manual registry file to update. +- Do not use `javax.inject.*` — this codebase is jakarta-based like the rest of the repo. +- Do not assume root `AGENTS.md`'s violation-fixer workflow covers this subproject's style checks. + +## Source of Truth + +This skill is the repository guidance for `grails-forge` work. When the module's conventions change, update this skill directly so agents load current rules from `.agents/skills/micronaut-developer/SKILL.md`. From 848cbee25b9e66b236a1b105c0b13b36ff7d2f3b Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 19:17:04 -0500 Subject: [PATCH 07/20] Add scoped grails-forge/AGENTS.md, link from root Root AGENTS.md (2073 words) was loaded in full for every grails-forge task despite being ~0% applicable: grails-forge is a Micronaut application (the generator behind start.grails.org), not a Grails app, and has its own settings.gradle/build. At least one root rule is actively wrong there - "@GrailsCompileStatic, not @CompileStatic" is inverted, since grails-forge correctly uses plain @CompileStatic (8 files, 0 @GrailsCompileStatic). Adds grails-forge/AGENTS.md (+ CLAUDE.md symlink, matching root's agent-neutral pattern) scoped to what's actually true here: corrected critical rules, this subproject's own build/test/style commands (verified against its CI job, .github/workflows/codestyle.yml's check_forge_projects), its module table, and a pointer to the micronaut-developer skill for code patterns. PR/branch/review/security policy is explicitly not repeated, to avoid drift from the root copy. Root AGENTS.md's Project Structure table now points to the nested file so an agent starting at repo root knows it exists. --- AGENTS.md | 4 +- grails-forge/AGENTS.md | 96 ++++++++++++++++++++++++++++++++++++++++++ grails-forge/CLAUDE.md | 1 + 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 grails-forge/AGENTS.md create mode 120000 grails-forge/CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md index a9af25dc8c2..5a527fed0a2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,9 +87,9 @@ This repository contains multiple independent Gradle projects: | **grails-core** (root) | Main framework with 60+ modules | `./gradlew build` | | **build-logic/** | Gradle convention plugins for the build | `cd build-logic && ../gradlew build` | | **grails-gradle/** | Grails Gradle plugins | `cd grails-gradle && ./gradlew build` | -| **grails-forge/** | Application generator (like Spring Initializr) | `cd grails-forge && ./gradlew build` | +| **grails-forge/** | Application generator (like Spring Initializr) — Micronaut, not Grails; see [`grails-forge/AGENTS.md`](grails-forge/AGENTS.md) | `cd grails-forge && ./gradlew build` | -Each project has its own `settings.gradle` and independent build. When working on a specific project, run Gradle commands from that project's directory. +Each project has its own `settings.gradle` and independent build. When working on a specific project, run Gradle commands from that project's directory. `grails-forge/` has its own nested `AGENTS.md` — read it before working there, since most of this file's Grails/GORM/Hibernate content doesn't apply to it. ## Dependency Management diff --git a/grails-forge/AGENTS.md b/grails-forge/AGENTS.md new file mode 100644 index 00000000000..7c75e2ba438 --- /dev/null +++ b/grails-forge/AGENTS.md @@ -0,0 +1,96 @@ + + +# Agent Guide for grails-forge + +> **IMPORTANT**: `grails-forge` is a **Micronaut application that generates Grails applications** +> (the engine behind start.grails.org) — it is not itself a Grails application. It has its own +> `settings.gradle` and builds independently of the rest of this repository. Most of the root +> [`AGENTS.md`](../AGENTS.md) — GORM, artefact handlers, Hibernate, `@GrailsCompileStatic` — does +> not apply here. This file covers what's different; the root file still governs PR/branch/review +> conventions and repository-wide policy, which are unchanged for this subproject. + +## Quick Reference + +Run from inside `grails-forge/`, not the repo root: + +```bash +# Build (no tests) +./gradlew build -PskipTests + +# Build a single module +./gradlew :grails-forge-core:build + +# Run tests +./gradlew :grails-forge-api:test +./gradlew :grails-forge-cli:test + +# Code style (Checkstyle + Spotless — this is what CI's "Forge Projects" job runs) +./gradlew codeStyle +``` + +## Critical Rules (corrected for this subproject) + +Root `AGENTS.md`'s rules assume a Grails application or the Grails framework itself. Here, checked against actual source: + +1. **Use `jakarta.*` NOT `javax.*`** — same as root, and it holds here too (Micronaut is jakarta-based). +2. **Use plain `@CompileStatic`, NOT `@GrailsCompileStatic`.** This inverts the root rule. There are no Grails artefacts in this codebase, so `@GrailsCompileStatic` doesn't apply and shouldn't appear — plain `@CompileStatic` is correct. +3. **No `GrailsWebRequest`, no GORM, no artefact handlers.** These concepts don't exist here. If you find yourself reaching for one, you're importing a mental model from the wrong subproject. +4. **Code style tooling is Checkstyle + Spotless**, not the root project's CodeNarc/PMD/SpotBugs/`aggregateViolations` stack. Run `./gradlew codeStyle` from `grails-forge/`, not root's `aggregateViolations`. +5. **Dependency versions are independent of `grails-bom`.** This subproject has its own `gradle.properties` (`micronautVersion`, `picocliVersion`, etc.) and isn't governed by the root `validateDependencyVersions` check. +6. **Apache license header is still required** on every new source file — this rule is unchanged from root. +7. **4 spaces, no tabs** — also unchanged from root. + +## Available Skills + +Same directory-based discovery as root: list `../.agents/skills/*/SKILL.md`, read each front-matter `description`, load the ones that match. The skill most relevant to this subproject is `micronaut-developer`, which covers DI, HTTP controllers, `@MicronautTest`+Spock, Picocli CLI commands, Rocker templating, and the `Feature` extension-point system in depth — read it before making non-trivial changes here. + +```bash +for f in ../.agents/skills/*/SKILL.md; do awk -F': *' '/^description:/{print FILENAME": "$2; exit}' "$f"; done +``` + +## Technology Stack + +| Component | Version | +|---|---| +| Micronaut | 4.10.16 | +| Picocli | 4.7.6 | +| JDK | 21+ | +| Testing | Spock via `@MicronautTest` | +| Templating | Rocker (`.rocker.raw`) | +| Code style | Checkstyle + Spotless | + +## Project Structure + +| Module | Role | +|---|---| +| `grails-forge-core` | Generation logic: the `Feature` system, templating, dependency/config assembly | +| `grails-forge-api` | HTTP API (Micronaut `@Controller`s) — backs start.grails.org | +| `grails-forge-web-netty` | Micronaut/Netty deployment of the API, shipped to Google Cloud Run | +| `grails-forge-cli` | Picocli command-line client | +| `grails-forge-analytics-postgres` | Separate Postgres-backed analytics service | +| `test-core` | End-to-end specs that generate a project and verify it builds | +| `grails-cli`, `grails-cli-shadow` | Legacy shell-packaging plumbing, unrelated to the generator logic | + +See the `micronaut-developer` skill for code patterns (DI, controllers, testing, CLI, templating, the `Feature` interface) — this table is orientation only. + +## CI + +The `check_forge_projects` job ("Forge Projects") in `.github/workflows/codestyle.yml` runs `./gradlew codeStyle` from `grails-forge/` independently of the root project's style checks. There's a separate `.github/workflows/gradle.yml` job for the actual build/test matrix, and dedicated `forge-deploy-*.yml` workflows handle Cloud Run deployment (snapshot/prev/release channels) — none of which touch the rest of the monorepo. + +## What's Unchanged From Root AGENTS.md + +PR guidelines, branch naming, review process, adversarial self-review, security reporting, and the single-large-PR-over-reviewability-stack default all apply here exactly as documented in [`../AGENTS.md`](../AGENTS.md) — this file doesn't repeat them so they can't drift out of sync with the root copy. Read that file for anything not covered above. diff --git a/grails-forge/CLAUDE.md b/grails-forge/CLAUDE.md new file mode 120000 index 00000000000..47dc3e3d863 --- /dev/null +++ b/grails-forge/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file From 865b0ef37831cda3acf1b81453c23cffc0abd402 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 19:20:37 -0500 Subject: [PATCH 08/20] Drop grails-forge/CLAUDE.md symlink The root CLAUDE.md -> AGENTS.md symlink exists because some tools hard-require that exact filename; replicating it in every subproject with a nested AGENTS.md doesn't scale and shouldn't be necessary if AGENTS.md is discovered natively in nested directories. About to test that assumption directly rather than assume it. --- grails-forge/CLAUDE.md | 1 - 1 file changed, 1 deletion(-) delete mode 120000 grails-forge/CLAUDE.md diff --git a/grails-forge/CLAUDE.md b/grails-forge/CLAUDE.md deleted file mode 120000 index 47dc3e3d863..00000000000 --- a/grails-forge/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -AGENTS.md \ No newline at end of file From 215d68dba319dcef974bd1373b0ea466f71983be Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 19:52:08 -0500 Subject: [PATCH 09/20] Document the Feature-testing pattern the skill was missing An A/B test (agent working in grails-forge with vs without the scoped guidance, identical prompt) surfaced a real gap: this skill documented @MicronautTest+Spock for HTTP controllers but had nothing on ApplicationContextSpec, the different, lighter-weight pattern actually used to test Feature classes (no server startup, just a shared ApplicationContext). Both test runs had to reverse-engineer it from MongoSyncSpec.groovy; the "after" agent's own report even claimed this was "the pattern the skill describes" - it wasn't, grep confirmed zero hits for ApplicationContextSpec/CommandOutputFixture/BuildBuilder before this commit. Adds the pattern grounded in the real source (ApplicationContextSpec, CommandOutputFixture, BuildBuilder, MongoSyncSpec) and clarifies the two testing patterns aren't interchangeable - MicronautTest is for grails-forge-api controllers, ApplicationContextSpec is for grails-forge-core Feature classes. --- .agents/skills/micronaut-developer/SKILL.md | 58 +++++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/.agents/skills/micronaut-developer/SKILL.md b/.agents/skills/micronaut-developer/SKILL.md index 31f0a9b915d..834dc1d28bd 100644 --- a/.agents/skills/micronaut-developer/SKILL.md +++ b/.agents/skills/micronaut-developer/SKILL.md @@ -15,7 +15,7 @@ limitations under the License. --> --- name: micronaut-developer -description: Guide for working in grails-forge (grails-forge-core, grails-forge-api, grails-forge-cli, grails-forge-web-netty) — a Micronaut application, not a Grails one. Covers Micronaut DI/bean patterns, HTTP controllers, MicronautTest+Spock, Picocli CLI commands, Rocker templating, and the Feature extension-point system. Use this instead of grails-developer/hibernate-developer when changing code under grails-forge/. +description: Guide for working in grails-forge (grails-forge-core, grails-forge-api, grails-forge-cli, grails-forge-web-netty) — a Micronaut application, not a Grails one. Covers Micronaut DI/bean patterns, HTTP controllers, two distinct Spock testing patterns (MicronautTest for HTTP controllers, ApplicationContextSpec for Feature classes), Picocli CLI commands, Rocker templating, and the Feature extension-point system. Use this instead of grails-developer/hibernate-developer when changing code under grails-forge/. license: Apache-2.0 compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf metadata: @@ -26,7 +26,7 @@ metadata: ## What I Do - Provide repository-specific guidance for `grails-forge/` — the project generator behind start.grails.org (the Grails equivalent of Spring Initializr). -- Cover Micronaut idioms actually used in this codebase: DI (`@Singleton`, `@Inject`), HTTP (`@Controller`, `@Get`, `@Post`), testing (`@MicronautTest` + Spock), and bean indexing for plugin-style extension points (`@Indexed`). +- Cover Micronaut idioms actually used in this codebase: DI (`@Singleton`, `@Inject`), HTTP (`@Controller`, `@Get`, `@Post`), the two distinct Spock testing patterns (`@MicronautTest` for HTTP controllers, `ApplicationContextSpec` for `Feature` classes — don't conflate them), and bean indexing for plugin-style extension points (`@Indexed`). - Cover the Picocli CLI layer in `grails-forge-cli` and the Rocker templating engine used for both code generation output and API responses. - Correct root `AGENTS.md` rules that do not apply here — see "Where Root AGENTS.md Rules Don't Apply" below. @@ -114,9 +114,9 @@ public class ApplicationController { } ``` -### Testing: MicronautTest + Spock +### Testing HTTP Controllers: MicronautTest + Spock -Not `HibernateGormDatastoreSpec` or any grails-core testing-support class — full HTTP round-trip tests via an injected client: +For `grails-forge-api` controllers specifically. Not `HibernateGormDatastoreSpec` or any grails-core testing-support class — full HTTP round-trip tests via an injected client: ```groovy @MicronautTest @@ -134,6 +134,56 @@ class ApplicationControllerSpec extends Specification { } ``` +### Testing Feature Classes: ApplicationContextSpec (not MicronautTest) + +For `grails-forge-core` `Feature` implementations, this is a **different, lighter-weight pattern** from the HTTP-controller one above — no full server startup, just a real Micronaut `ApplicationContext`. Don't reach for `@MicronautTest` here; it's the wrong tool for testing generation logic. + +`ApplicationContextSpec` (`grails-forge-core/src/test/groovy/org/grails/forge/ApplicationContextSpec.groovy`) is a plain Spock `Specification`, not `@MicronautTest`-annotated, that spins up a shared context manually: + +```groovy +abstract class ApplicationContextSpec extends Specification implements ProjectFixture, ContextFixture { + Map getConfiguration() { [:] } + + @Shared + @AutoCleanup + ApplicationContext beanContext = ApplicationContext.run(configuration) +} +``` + +A feature spec extends it and mixes in `CommandOutputFixture` for generated-file assertions: + +```groovy +class MongoSyncSpec extends ApplicationContextSpec implements CommandOutputFixture { + + void 'test readme.md with feature mongo-sync contains links to micronaut and 3rd party docs'() { + when: + def output = generate(['mongo-sync']) + then: + output["README.md"].contains("https://www.mongodb.com/docs/drivers/java/sync/current/") + } + + void "test mongo sync features"() { + when: + Features features = getFeatures(['mongo-sync']) + then: + features.contains("mongo-sync") + } + + void "test mongo sync dependencies are present for gradle"() { + when: + String template = new BuildBuilder(beanContext).features(["mongo-sync"]).render() + then: + template.contains('implementation "org.mongodb:mongodb-driver-sync"') + } +} +``` + +Three distinct helpers, each answering a different question about a feature — use whichever fits what you're actually asserting: + +- **`generate(List features)`** (from `CommandOutputFixture`) — generates a full project, returns `Map` of file path → content. Use for README/scaffolding-output assertions. +- **`getFeatures(List features)`** (from `ContextFixture`, mixed into `ApplicationContextSpec`) — returns the resolved `Features` collection. Use to assert a feature applies/registers correctly. +- **`new BuildBuilder(beanContext).features([...]).render()`** — renders just the generated `build.gradle` content as a string. Use for dependency/build-file assertions without generating a whole project. + ### CLI Commands (Picocli, DI-wired) `grails-forge-cli` commands are Picocli `@CommandLine.Command` classes, DI-constructed: From 6fa20d9d374a5748066bec20fff66e915ad66886 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 20:13:56 -0500 Subject: [PATCH 10/20] Document coverage visibility - Codecov data is local, not CI-only Answers a real question: the repo has Codecov integrated (codecov.yml, coverage.yml), but nothing said whether that data is visible without a CI round-trip. Checked the actual task sources (GrailsJacocoPlugin, GrailsViolationAggregationPlugin in build-logic): - aggregateJacocoCoverage -> JACOCO_COVERAGE.md (already used by violation-fixer, just never connected to "Codecov" by name) - jacocoAggregateReport -> the exact XML CI uploads to Codecov Both are ordinary local Gradle tasks, no token or network needed. What's actually CI-only is Codecov's diff-against-base-branch comparison, PR comment, and dashboard - and per codecov.yml both status checks are informational: true, so they don't block merge today regardless. Also notes coverage.yml only wires up grails-core and grails-gradle - grails-forge and build-logic have no Codecov data at all, in CI or locally. --- AGENTS.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 5a527fed0a2..3683c2f9b01 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -222,6 +222,21 @@ class MyService { } | Build docs | `./gradlew :grails-doc:publishGuide -x aggregateGroovydoc` | | Debug | `./gradlew bootRun --debug-jvm` | +## Coverage + +Coverage is uploaded to Codecov (`codecov.yml`), but the underlying data is fully visible locally — it does not require CI: + +| Task | Output | Scope | +|------|--------|-------| +| `./gradlew aggregateJacocoCoverage` | `build/reports/violations/JACOCO_COVERAGE.md` | Human-readable per-module Markdown table | +| `./gradlew jacocoAggregateReport` | `build/reports/jacoco/aggregate/jacocoAggregateReport.xml` | The exact XML CI uploads to Codecov — same data, same command, runnable locally | + +Both are ordinary local Gradle tasks (`GrailsJacocoPlugin`/`GrailsViolationAggregationPlugin` in `build-logic`); neither needs network access or a Codecov token. Run either before committing to see current coverage — don't wait on a CI round-trip to find out. + +What genuinely requires CI/Codecov's cloud service (not reproducible locally): the diff-coverage comparison against the PR's base branch, the PR comment Codecov posts, and the codecov.io dashboard/badge. Per `codecov.yml`, both the `patch` and `project` status checks are `informational: true` — they don't block merge today, so a red Codecov check is a signal to look at, not a hard gate. + +`grails-forge/` and `build-logic/` are not wired into `coverage.yml` (only `grails-core` and `grails-gradle` are) — no Codecov data exists for them either locally or in CI. + ## Branch Naming (Auto-Labels PRs) | Prefix | Label | From 41fa17b08cb8328b2f0389c3695c52f8a0623cda Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 20:29:49 -0500 Subject: [PATCH 11/20] Add diff-coverage-check skill: real diff coverage, fully local Diff coverage (coverage of only the lines you actually changed) is what Codecov computes in CI, but the underlying data - JaCoCo line hit/miss counters - is already produced locally by an ordinary :module:test run (GrailsJacocoPlugin, finalizedBy jacocoTestReport). The only missing piece was cross-referencing it against git diff. Considered wiring in form-com/diff-coverage-gradle (a real, maintained plugin for exactly this) but that's a build.gradle change with its own review burden and a new external dependency - a different risk category from the rest of this branch. This skill gets the same signal with zero build changes: run the affected module's tests, then cross-reference its existing JaCoCo XML against git diff hunks. The script and procedure are verified against real data in this repo, not assumed from JaCoCo's schema docs: - Confirmed module derivation must walk up to the nearest build.gradle, not assume module = first path segment (grails-gsp/grails-taglib and grails-gsp/core are separate modules nested two levels deep). - Confirmed some .java files live under src/main/groovy/, not src/main/java/ (grails-i18n/.../AvailableLocaleResolver.java). - Found and documented a real gotcha: a stale JaCoCo report doesn't error, it silently cross-references the wrong code against shifted line numbers - must regenerate after every edit. - Verified end-to-end against a real mixed-coverage diff in grails-i18n (one covered line, one genuinely uncovered dead branch), confirming the script correctly identifies exactly which changed line needs a test. --- .agents/skills/diff-coverage-check/SKILL.md | 138 ++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 .agents/skills/diff-coverage-check/SKILL.md diff --git a/.agents/skills/diff-coverage-check/SKILL.md b/.agents/skills/diff-coverage-check/SKILL.md new file mode 100644 index 00000000000..86ab9a82b8f --- /dev/null +++ b/.agents/skills/diff-coverage-check/SKILL.md @@ -0,0 +1,138 @@ + +--- +name: diff-coverage-check +description: Computes real diff coverage (coverage of only the lines you actually changed, not whole-file coverage) entirely locally, without CI or Codecov — by running each affected module's own tests and cross-referencing its JaCoCo XML against git diff. Use before committing, or when asked "is my change covered" / "check coverage on the files I touched" / "diff coverage". A change often spans several modules; run this per module. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +metadata: + audience: maintainers + frameworks: grails +--- + +## What I Do + +- Compute diff coverage — coverage of the specific lines changed in a diff, not whole-file/whole-class coverage — using only local tooling already wired into this repo's build. No new Gradle plugin, no Codecov token, no network. +- Run the actual test suite for each module touched by the change (not the whole repo), so this stays fast and scoped instead of a full aggregate build. +- Cross-reference each module's standard JaCoCo XML report (`/build/reports/jacoco/test/jacocoTestReport.xml`, produced automatically whenever `:module:test` runs — see `GrailsJacocoPlugin`, `finalizedBy('jacocoTestReport')`) against the exact line ranges `git diff` reports as changed. + +## Why This Exists + +Codecov (`codecov.yml`) computes diff coverage in CI, but that's a cloud round-trip. The underlying data — JaCoCo line-level hit/miss counters — is already produced locally by an ordinary `:module:test` run; the only missing piece was cross-referencing it against `git diff`. That's what this skill does, without adding any build dependency (a Gradle plugin for this, `form-com/diff-coverage-gradle`, exists and was considered, but wiring in a new external plugin/dependency is a real build change with its own review burden — this skill gets the same signal without touching `build.gradle` at all). + +## When to Use Me + +- Before committing, to check whether the lines you just wrote/changed are actually exercised by tests — not just "does the module's overall coverage look okay." +- When asked to check coverage on specific touched files, or "diff coverage" generally. +- A single change frequently spans multiple modules in this monorepo — expect to loop the procedure below once per affected module, not just once. + +## Procedure + +### 1. Find which files changed, and group them by Gradle module + +```bash +git diff --name-only -- '*.java' '*.groovy' +``` + +For each changed file, find its owning Gradle module by walking up from the file to the **nearest ancestor directory containing a `build.gradle`** — do not assume module = the first path segment. This repo has modules nested two levels deep (e.g. `grails-gsp/grails-taglib` and `grails-gsp/core` are separate modules, both under `grails-gsp/`, which itself has no `build.gradle`). The Gradle project path is that directory's path relative to repo root with `/` replaced by `:`, prefixed with `:` (e.g. `grails-gsp/grails-taglib` → `:grails-gsp:grails-taglib`). + +### 2. For each affected module, run its tests + +```bash +./gradlew :grails-gsp:grails-taglib:test +``` + +This auto-triggers `jacocoTestReport` afterward (wired via `finalizedBy` in `GrailsJacocoPlugin` — you don't need to call it separately). **Always run this fresh, even if you think a report already exists.** A stale report will not error — it will silently cross-reference the wrong code. (Verified directly: editing a file shifted its line numbers, and the stale XML matched the edited line number against unrelated old-code coverage data until the report was regenerated. This fails silently, not loudly — always regenerate.) + +### 3. Locate the package/sourcefile for each changed file + +Strip the file's path down to whatever comes after `src/main/java/` or `src/main/groovy/` (check both — this repo puts some `.java` files under `src/main/groovy/`, e.g. `grails-i18n/src/main/groovy/org/grails/plugins/i18n/AvailableLocaleResolver.java`). The directory portion (with `/`, not `.`) is the JaCoCo `package` name; the filename is the `sourcefile` name. + +### 4. Cross-reference changed lines against the JaCoCo XML + +```python +#!/usr/bin/env python3 +import re, subprocess, sys +import xml.etree.ElementTree as ET + +def changed_lines(filepath, base_ref): + diff = subprocess.run( + ['git', 'diff', '--unified=0', base_ref, '--', filepath], + capture_output=True, text=True, check=True + ).stdout + lines = set() + for m in re.finditer(r'^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@', diff, re.MULTILINE): + start = int(m.group(1)) + count = int(m.group(2)) if m.group(2) is not None else 1 + if count == 0: + continue # pure deletion, nothing added in the new file + lines.update(range(start, start + count)) + return lines + +def jacoco_line_coverage(xml_path, package, sourcefile): + root = ET.parse(xml_path).getroot() + for pkg in root.iter('package'): + if pkg.get('name') != package: + continue + for sf in pkg.iter('sourcefile'): + if sf.get('name') != sourcefile: + continue + return {int(l.get('nr')): (int(l.get('mi')), int(l.get('ci'))) for l in sf.iter('line')} + return {} + +def report(filepath, xml_path, package, sourcefile, base_ref): + changed = changed_lines(filepath, base_ref) + cov = jacoco_line_coverage(xml_path, package, sourcefile) + instrumented = {ln: cov[ln] for ln in changed if ln in cov} + covered = {ln for ln, (mi, ci) in instrumented.items() if ci > 0} + missed = sorted(ln for ln, (mi, ci) in instrumented.items() if ci == 0 and mi > 0) + if not instrumented: + print(f"{filepath}: no instrumented lines in this diff (comments/blank lines/braces only, or file not found in report)") + return + pct = 100 * len(covered) / len(instrumented) + print(f"{filepath}: {len(covered)}/{len(instrumented)} changed+instrumented lines covered ({pct:.1f}%)") + if missed: + print(f" Uncovered changed lines: {missed}") + +if __name__ == '__main__': + # filepath, xml_path, package, sourcefile, base_ref + report(*sys.argv[1:6]) +``` + +Example invocation, for a change to `grails-i18n/src/main/groovy/org/grails/plugins/i18n/AvailableLocaleResolver.java` against branch `8.0.x`: + +```bash +python3 diff_coverage.py \ + grails-i18n/src/main/groovy/org/grails/plugins/i18n/AvailableLocaleResolver.java \ + grails-i18n/build/reports/jacoco/test/jacocoTestReport.xml \ + org/grails/plugins/i18n \ + AvailableLocaleResolver.java \ + 8.0.x +``` + +### 5. Repeat per module, then summarize + +Loop steps 1–4 for every module the diff touches, then report a combined summary. Don't stop after the first module — "it might take several modules to get it done" is the normal case for this monorepo, not the exception. + +## Interpreting Results + +- **Uncovered changed lines are exactly what to add tests for** — that's the actionable output, more precise than a whole-class coverage percentage. +- **"No instrumented lines"** for a changed file usually means the diff only touched comments, imports, blank lines, or braces — not a problem, just nothing for JaCoCo to measure. +- A completely new, never-executed class still appears in the JaCoCo XML (with every line `ci=0`) rather than being silently absent — JaCoCo reports on all compiled classes in the module's `classDirectories`, not just ones a specific test run happened to exercise. So a brand-new untested class will correctly show as 0% covered, not "not found." + +## Source of Truth + +This skill's script and procedure were verified against a real JaCoCo report and a real git diff in this repository (`grails-i18n`), not assumed from JaCoCo's documented schema alone. If `GrailsJacocoPlugin`'s report locations or the JaCoCo XML schema version change, re-verify before trusting this skill's output. From f215aad82ed37f5d7026f548224e9c212d61a31c Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 20:32:50 -0500 Subject: [PATCH 12/20] Wire diff-coverage-check into the PR checklist, fix stale claim Two problems the previous commit left behind: 1. The Coverage section still claimed diff-coverage "genuinely requires CI/Codecov's cloud service (not reproducible locally)" - wrong as of the diff-coverage-check skill added in the prior commit. Corrected. 2. The skill was only discoverable (an agent might find it via the mandatory pre-code skill scan, or if explicitly asked about coverage), never required. PR checklist step 5 still just said "ensure coverage" with no mechanism to check it. Now points directly at the skill. --- AGENTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3683c2f9b01..79872d58d35 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -233,7 +233,7 @@ Coverage is uploaded to Codecov (`codecov.yml`), but the underlying data is full Both are ordinary local Gradle tasks (`GrailsJacocoPlugin`/`GrailsViolationAggregationPlugin` in `build-logic`); neither needs network access or a Codecov token. Run either before committing to see current coverage — don't wait on a CI round-trip to find out. -What genuinely requires CI/Codecov's cloud service (not reproducible locally): the diff-coverage comparison against the PR's base branch, the PR comment Codecov posts, and the codecov.io dashboard/badge. Per `codecov.yml`, both the `patch` and `project` status checks are `informational: true` — they don't block merge today, so a red Codecov check is a signal to look at, not a hard gate. +Diff coverage (coverage of only the lines you changed, which is what Codecov's PR comment shows) is also reproducible locally — see the `diff-coverage-check` skill, which cross-references a module's JaCoCo XML against `git diff`. What's still cloud-only: the PR comment itself, and the codecov.io dashboard/badge. Per `codecov.yml`, both the `patch` and `project` status checks are `informational: true` — they don't block merge today, so a red Codecov check is a signal to look at, not a hard gate. `grails-forge/` and `build-logic/` are not wired into `coverage.yml` (only `grails-core` and `grails-gradle` are) — no Codecov data exists for them either locally or in CI. @@ -272,7 +272,7 @@ work. 2. **Run tests** before submitting: `./gradlew build --rerun-tasks` 3. **Run code style checks**: `./gradlew codeStyle` 4. **Clean violations**: Before committing, run `./gradlew clean aggregateViolations` from the root and ensure that `build/reports/violations/CHECKSTYLE_VIOLATIONS.md`, `build/reports/violations/CODENARC_VIOLATIONS.md`, `build/reports/violations/PMD_VIOLATIONS.md`, and `build/reports/violations/SPOTBUGS_VIOLATIONS.md` have no issues. -5. **Verify test coverage**: Ensure any touched class is covered by tests verifying all behavior. You must run ALL tests in the affected module(s) and ensure they pass before submission. +5. **Verify test coverage**: Ensure any touched class is covered by tests verifying all behavior. You must run ALL tests in the affected module(s) and ensure they pass before submission. Run the `diff-coverage-check` skill against your changed files before submitting — don't rely on "the class has some tests" as a proxy for "the lines I changed are covered." 6. **Adversarial self-review (always, regardless of PR size)**: Run an adversarial review pass (e.g. `/code-review`, or a fresh-context agent) against this file's rules — jakarta not javax, no wildcard imports, BOM version rules, test coverage — and against the change's own logic. Do this for every PR, including large/consolidated ones; do not rely on GitHub's Copilot reviewer alone, since it silently skips PRs over ~300 files. Fix or flag anything it finds. This is a supplement to human review, not a replacement for it. 7. **Squash commits** into a single meaningful commit message 8. **Reference issues** in PR description (e.g., "Fixes #1234") From 95103ec475acb1c7536aa818d2fe867be1a245fd Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 20:35:12 -0500 Subject: [PATCH 13/20] Cross-link test-fixer and diff-coverage-check Same gap as the previous commit, mirrored: test-fixer already existed for diagnosing test failures, but PR checklist step 2 ("Run tests") never pointed to it, and neither skill referenced the other despite being sequential steps in the same workflow (run tests -> fix failures via test-fixer -> once passing, check coverage via diff-coverage-check). A failing test run also produces an untrustworthy JaCoCo report, so the ordering matters, not just the pointer. --- .agents/skills/diff-coverage-check/SKILL.md | 2 ++ .agents/skills/test-fixer/SKILL.md | 2 ++ AGENTS.md | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.agents/skills/diff-coverage-check/SKILL.md b/.agents/skills/diff-coverage-check/SKILL.md index 86ab9a82b8f..a8db03279eb 100644 --- a/.agents/skills/diff-coverage-check/SKILL.md +++ b/.agents/skills/diff-coverage-check/SKILL.md @@ -39,6 +39,8 @@ Codecov (`codecov.yml`) computes diff coverage in CI, but that's a cloud round-t - When asked to check coverage on specific touched files, or "diff coverage" generally. - A single change frequently spans multiple modules in this monorepo — expect to loop the procedure below once per affected module, not just once. +This skill assumes the module's tests already pass. If `:module:test` fails, that's a different problem — use the `test-fixer` skill first; a failing test run won't produce a trustworthy JaCoCo report either. + ## Procedure ### 1. Find which files changed, and group them by Gradle module diff --git a/.agents/skills/test-fixer/SKILL.md b/.agents/skills/test-fixer/SKILL.md index a34ca8ccb71..72a1cb2d7f2 100644 --- a/.agents/skills/test-fixer/SKILL.md +++ b/.agents/skills/test-fixer/SKILL.md @@ -24,6 +24,8 @@ Activate this skill when: - Triaging regressions after a dependency upgrade or refactor. - Reviewing aggregate test results from `:grails-test-report`. +Once tests pass, that doesn't mean your changed lines are covered — see the `diff-coverage-check` skill for that; it's a separate question this skill doesn't answer. + --- ## Key Tasks diff --git a/AGENTS.md b/AGENTS.md index 79872d58d35..baba61e69b7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -269,7 +269,7 @@ work. ### Checklist 1. **Fork & branch** from the target release branch (e.g., `7.0.x`) -2. **Run tests** before submitting: `./gradlew build --rerun-tasks` +2. **Run tests** before submitting: `./gradlew build --rerun-tasks`. If anything fails, use the `test-fixer` skill rather than guessing. 3. **Run code style checks**: `./gradlew codeStyle` 4. **Clean violations**: Before committing, run `./gradlew clean aggregateViolations` from the root and ensure that `build/reports/violations/CHECKSTYLE_VIOLATIONS.md`, `build/reports/violations/CODENARC_VIOLATIONS.md`, `build/reports/violations/PMD_VIOLATIONS.md`, and `build/reports/violations/SPOTBUGS_VIOLATIONS.md` have no issues. 5. **Verify test coverage**: Ensure any touched class is covered by tests verifying all behavior. You must run ALL tests in the affected module(s) and ensure they pass before submission. Run the `diff-coverage-check` skill against your changed files before submitting — don't rely on "the class has some tests" as a proxy for "the lines I changed are covered." From d6915e1c3f9ce944b7f56e9601e4785da7d3fdd8 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 20:39:04 -0500 Subject: [PATCH 14/20] Add structural paths: scoping to module-specific skills hibernate-developer's own description already said "use this when changing code under grails-data-hibernate7" - but that's prose an agent has to interpret, not a structural signal, so a task could plausibly load it (or fail to) based on how well the description matched the agent's phrasing of its own task. Adds an optional paths: glob to front-matter (Agent Skills Specification allows arbitrary metadata) and teaches the directory- based discovery instruction in AGENTS.md to check it: if the file(s) being touched match a skill's paths, load it regardless of whether the task description alone would have matched. This is deliberately generic, not a hibernate-only fix - it's the same mechanism fix #4 already established (directory is the source of truth, not a hand-maintained list), just extended to path-scope in addition to semantic description matching. Applied to all three currently path-scoped skills to prove it generalizes: hibernate-developer (grails-data-hibernate7/**), micronaut-developer (grails-forge/**), worktree-hygiene (.claude/worktrees/**). Repo-wide skills (grails-developer, groovy-developer, etc.) intentionally have no paths: field. --- .agents/skills/hibernate-developer/SKILL.md | 1 + .agents/skills/micronaut-developer/SKILL.md | 1 + .agents/skills/worktree-hygiene/SKILL.md | 1 + AGENTS.md | 6 ++++-- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.agents/skills/hibernate-developer/SKILL.md b/.agents/skills/hibernate-developer/SKILL.md index ae3197ba976..d6d958af393 100644 --- a/.agents/skills/hibernate-developer/SKILL.md +++ b/.agents/skills/hibernate-developer/SKILL.md @@ -2,6 +2,7 @@ name: hibernate-developer description: Guide for working in the grails-data-hibernate7 module, especially Hibernate 7 domain binding, mapping migration, generators, and integration tests. Use this when changing code or tests under grails-data-hibernate7. license: Apache-2.0 +paths: grails-data-hibernate7/** --- + +## What I Do + +- Provide repository-specific guidance for the `grails-data-mongodb` project (GORM for MongoDB — a document-database GORM implementation, not a relational one; do not bring Hibernate/binder mental models here). +- Guide changes around `MongoMappingContext`, `BsonPersistentEntityCodec`/`PersistentEntityCodec`, and the codec-registry-based encode/decode pipeline. +- Keep changes aligned with the Testcontainers-backed testing pattern this module actually uses. + +## When to Use Me + +Activate this skill when working on the MongoDB module, especially for: + +- Changes under `grails-data-mongodb/**` (any of its 8 subprojects — see Module Structure). +- Document/collection mapping, codec, or GeoJSON-type work. +- Index (including TTL) creation/reconciliation logic. +- Multi-tenancy or multi-connection MongoDB work. + +## Module Context + +`grails-data-mongodb` has no `settings.gradle` of its own — it's part of the root aggregate build, same as `grails-data-hibernate7`. All of root `AGENTS.md`'s rules (jakarta, `@GrailsCompileStatic`, CodeNarc/Checkstyle/PMD/SpotBugs, `dependencies.gradle`/BOM) apply here unmodified. + +Eight Gradle projects live under this directory, mapped to real Gradle project names (not always `grails-data-mongodb-` — check before assuming): + +| Directory | Gradle project | Role | +|---|---|---| +| `core/` | `:grails-data-mongodb-core` | The actual GORM-for-MongoDB implementation: `MongoDatastore`, `MongoMappingContext`, `MongoQuery`, GeoJSON types | +| `bson/` | `:grails-data-mongodb-bson` | Low-level, GORM-independent BSON codec machinery (`BsonPersistentEntityCodec`, per-type property codecs, hand-rolled JSON tokenizer) | +| `ext/` | `:grails-data-mongodb-ext` | A single file, Groovy extension methods on the raw MongoDB driver (`distinct`, `watch`/change streams, `deleteMany`) | +| `boot-plugin/` | `:grails-data-mongodb-spring-boot` | Spring Boot auto-configuration (`MongoDbGormAutoConfiguration`) | +| `spring-data/` | `:grails-data-mongodb-spring-data` | Lets GORM-for-MongoDB and Spring Data MongoDB share a `MongoClient`/codecs/transaction | +| `grails-plugin/` | `:grails-data-mongodb` (note: not `-grails-plugin` suffixed) | Classic Grails plugin descriptor (`MongodbGrailsPlugin`); also ships the app-facing `grails.test.mongodb.MongoSpec` test base via `testFixtures` | +| `gson-templates/` | `:grails-data-mongodb-gson-templates` | JSON Views (`.gson`) for `ObjectId` and GeoJSON types | +| `docs/` | `:grails-data-mongodb-docs` | Asciidoc manual + groovydoc aggregation only, no source | + +## Key Classes and Responsibilities + +There is no single `GrailsDomainBinder`-equivalent class — the responsibility splits across three cooperating layers: + +### Mapping (the binder equivalent) + +- `MongoMappingContext` (`core/.../mongo/config/MongoMappingContext.java`) — owns the private inner `MongoDocumentMappingFactory extends AbstractGormMappingFactory`, which is the actual binder: forces the `_id` field name, applies the global `stringIdDefaultStoredAs` default, wires codec-registry-backed custom types, and registers all GeoJSON custom types. +- `MongoCollection` (`core/.../config/MongoCollection.groovy`) — entity-level mapping (collection name, database, `writeConcern`, indices, sort). +- `MongoAttribute` (`core/.../config/MongoAttribute.groovy`) — property-level mapping (`reference` for DBRef vs. embed, `field`, `geoIndex`, `index`, `indexAttributes`). +- `MappingBuilder.document { ... }` (`core/.../grails/mongodb/mapping/MappingBuilder.groovy`) — the programmatic DSL entry point. + +### Codec pipeline (the runtime read/write engine) + +- `BsonPersistentEntityCodec` (`bson/.../BsonPersistentEntityCodec.groovy`) and its Mongo subclass `PersistentEntityCodec` (`core/.../engine/codecs/PersistentEntityCodec.groovy`) — a static `ENCODERS`/`DECODERS` registry keyed by `PersistentProperty` subtype (`Identity`, `TenantId`, `Simple`, `Embedded`, `EmbeddedCollection`, `Custom`, `Basic`, plus Mongo-specific `OneToOne`/`ManyToOne`/`OneToMany`/`ManyToMany`). `encode`/`decode` walk `entity.persistentProperties` and dispatch per property kind. `PersistentEntityCodec` also implements MongoDB's partial-update path (`encodeUpdate`) via `DirtyCheckable`, producing `$set`/`$unset` documents directly rather than rewriting the whole document. + +### Datastore + +- `MongoDatastore` (`core/.../mongo/MongoDatastore.java`) — owns index creation/reconciliation against live MongoDB and transaction-capability detection based on cluster topology. + +## Confirmed MongoDB-Specific Concepts (implemented — don't assume beyond this list) + +- **Embedded documents & embedded collections** — `Embedded`/`EmbeddedCollection` property kinds, with dedicated in-place `$set`/`$unset` update logic distinct from top-level property updates. +- **GeoJSON types** — `grails.mongodb.geo.{Point,LineString,Polygon,MultiPoint,MultiLineString,MultiPolygon,GeometryCollection,Box,Circle,Sphere,Shape}`, each with a custom-type marshaller. Query support: `near`, `nearSphere`, `withinCircle`, `withinBox`, `withinPolygon`, `geoWithin`, `geoIntersects`. Legacy `2d`/`2dsphere` indexes via `MongoAttribute.geoIndex(String)`. +- **TTL indexes** — property-level `indexAttributes: [expireAfterSeconds: N]` materializes a MongoDB TTL index, reconciled in place via `collMod` rather than drop/rebuild (see Pitfalls). +- **DBRef vs. embedding** — `MongoAttribute.reference` toggles whether a `ToOne`/`ToMany` association is inline-embedded or a `com.mongodb.DBRef`. +- **Schemaless/dynamic attributes** — `DynamicAttributes` trait support baked into `BsonPersistentEntityCodec` and `MongoEntity`. +- **Change streams** — `MongoExtensions.watch(...)`, a thin Groovy-extension wrapper over the driver's native `watch()`, not a GORM-level abstraction. +- **Multi-document transactions** — opt-in via `grails.mongodb.transactional`, gated on cluster topology (`REPLICA_SET`/`SHARDED`/`LOAD_BALANCED` supported; `STANDALONE` falls back with a one-time warning). +- **Multi-tenancy / multiple named connections** — supported (discriminator-based multi-tenancy, `grails.mongodb.connections`). + +### Not implemented — do not document or rely on these + +- **`shard "name"` in the mapping DSL is a no-op.** There is no `shard` method on `Collection`/`MongoCollection`. The call silently falls through to the generic `Entity.methodMissing` catch-all, which just records it as an ad hoc property config — it never issues a MongoDB `shardCollection` command. Verified: zero `shardCollection` calls anywhere in `core/src/main`. If you see `shard "..."` in a mapping block, treat it as dead syntax, not a real feature. +- **Capped collections** — not implemented anywhere in this module. + +## Testing Rules + +Real MongoDB via Testcontainers is used — there is no embedded/in-memory Mongo fallback anywhere in this module. Docker or Podman must be running on the host. + +- **`GrailsDataTckSpec`** is the dominant pattern (111+ specs) — the same shared TCK harness the Hibernate modules use (`grails-datamapping-tck`), parameterized with `GrailsDataMongoTckManager` (`core/src/test/groovy/.../GrailsDataMongoTckManager.groovy`), which starts a real `MongoDBContainer` in `setupSpec()`/stops it in `cleanupSpec()`, and drops all non-system databases after each test. + ```groovy + class DocumentMappingSpec extends GrailsDataTckSpec { + void setupSpec() { + manager.registerDomainClasses(CustomMapping) + } + void "test custom document mapping"() { ... } + } + ``` + Domain classes are top-level classes in the same spec file, registered via `manager.registerDomainClasses(...)` in `setupSpec()` — same convention as `HibernateGormDatastoreSpec`. +- **`AutoStartedMongoSpec`** (`grails-testing-support-mongodb`) — a lighter-weight base used by a handful of specs; a Spock global extension auto-injects `@Shared MongoDBContainer dbContainer` and auto-constructs a `@Shared MongoDatastore` field if present. +- **`grails.test.mongodb.MongoSpec`** — a separate, *app-facing* base class shipped via `testFixtures` in `grails-plugin/`, for Grails-application-level Mongo integration tests. Don't confuse it with the two internal patterns above — a spec being both a `MongoSpec` and an `AutoStartedMongoSpec` is explicitly forbidden. +- Default Testcontainers image: `mongo:7.0.19` (overridable via `-DmongodbContainerVersion=...`). +- The `core` module's tests run with **`maxParallelForks = 1`** (serial), via `gradle/mongodb-forked-test-config.gradle` — a deliberate constraint for Testcontainers-backed integration tests. Other subprojects (`bson`, `spring-data`, `boot-plugin`, `grails-plugin`, `gson-templates`) use the normal parallel `mongodb-test-config.gradle`. Don't "fix" `core`'s serial execution without understanding why it's there. +- `-PskipMongodbTests` / `-PonlyMongodbTests` gate these tests at the root, matching the `Container missing` row in root `AGENTS.md`'s Common Issues table — use `-PskipMongodbTests` when Docker isn't available. + +## Change Workflow + +1. Identify which layer owns the behavior: mapping (`MongoMappingContext`/`MongoCollection`/`MongoAttribute`) vs. codec/runtime (`BsonPersistentEntityCodec`/`PersistentEntityCodec`) vs. datastore-level (`MongoDatastore`, index/transaction handling). +2. Check whether the change affects the general `bson/` codec machinery (used by any BSON-backed consumer) or is genuinely Mongo-specific (`core/`) — don't put Mongo-only logic in `bson/`. +3. Update or add specs via `GrailsDataTckSpec`, registering any new domain classes as top-level classes in the same spec file. +4. Run the affected subproject's tests with Docker/Podman running; expect `core` tests to run serially. + +## Pitfalls to Avoid + +- Do not treat `shard "name"` as a functioning DSL keyword — it's a documented no-op (see above). Don't build new features assuming it works, and flag it if you're asked to "fix" sharding — the DSL surface exists but the implementation doesn't. +- Do not casually raise `core`'s `maxParallelForks = 1` — it's deliberate for Testcontainers stability, not an oversight. +- TTL index changes go through `collMod`, not drop/rebuild — a single-field `expireAfterSeconds` change should reconcile in place. MongoDB silently ignores the TTL option on a compound index; don't declare one there. +- The global `stringIds.defaultStoredAs` setting must be read by `MongoMappingContext` **before** `initialize(classes)` runs (`createIdentity` is invoked during entity registration and depends on it) — if you touch mapping-context initialization order, this dependency is easy to break silently. +- Known unaddressed gaps (real `// TODO`s in the encode/decode path, not exhaustive but worth checking before assuming behavior): unprocessed `OneToMany` associations in `encodeUpdate` (`PersistentEntityCodec.groovy`), `Map` handling in the embedded-collection update path, and embedded-collection support in the base `BsonPersistentEntityCodec.encodeUpdate` (only the Mongo subclass handles it). + +## Known Status and Constraints + +- Multi-document transactions require positive cluster-topology detection (`REPLICA_SET`/`SHARDED`/`LOAD_BALANCED`); a `STANDALONE` cluster falls back to legacy client-side flush behavior with a one-time warning, not a hard failure. +- `MongoDbGormAutoConfiguration` only closes a `MongoClient` it created itself, not one supplied as an external bean — preserve that ownership tracking if you touch auto-configuration shutdown logic. + +## Source of Truth + +This skill is the repository guidance for `grails-data-mongodb` work. When module conventions change, update this skill directly so agents load current rules from `.agents/skills/mongodb-developer/SKILL.md`. From 48d3e5b49ca0c9578429c31ff95a81a7909d224f Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 21:38:46 -0500 Subject: [PATCH 16/20] Add grails-gradle-developer and build-logic-developer skills + AGENTS.md Two more independent sub-builds with zero agent guidance, found the same way grails-forge was: own settings.gradle, own test harness, but neither fits the grails-forge template cleanly - both are hybrids that share root's dependencies.gradle/gradle.properties (via a purpose-built SharedPropertyPlugin that walks up the directory tree, since "Gradle can't share properties across buildSrc or composite projects") while diverging on everything else: Groovy 4.0.32 (Gradle's embedded version) instead of root's 5.0.x, plain @CompileStatic never @GrailsCompileStatic (verified: 0 real annotation usages in either module, only javadoc mentions), and Gradle TestKit (GradleRunner) instead of Spock/GORM-mock testing. grails-gradle: the actual Plugin classes an app's build.gradle applies (org.apache.grails.gradle.grails-app etc.) - catalogued all 12 plugin-ID-to-class mappings, the GradleSpecification TestKit base class, and the model/common/plugins/tasks/bom module split (spock excluded transitively via grails-gradle-model, verified). build-logic: the convention-plugin layer every other build in this repo (root, grails-forge, grails-gradle) consumes transitively via includeBuild - implements GrailsJacocoPlugin, GrailsViolationAggregationPlugin, and 11 others that other skills already cited by name without documentation. Catalogued all 13 plugins with what each actually configures, both testing patterns (TestKit for task-graph behavior, ProjectBuilder for pure logic), and flagged that only 4 of 13 plugins have dedicated specs. No CI job of its own is expected here, not a dormancy signal like grails-data-neo4j - confirmed live via a substantive commit (GROOVY-12146 workaround) the day before this was written, not just "exercised transitively" hand-waving. Root AGENTS.md's Project Structure table now points to both nested files, same pattern as grails-forge. --- .agents/skills/build-logic-developer/SKILL.md | 149 ++++++++++++++++++ .../skills/grails-gradle-developer/SKILL.md | 126 +++++++++++++++ AGENTS.md | 6 +- build-logic/AGENTS.md | 71 +++++++++ grails-gradle/AGENTS.md | 75 +++++++++ 5 files changed, 424 insertions(+), 3 deletions(-) create mode 100644 .agents/skills/build-logic-developer/SKILL.md create mode 100644 .agents/skills/grails-gradle-developer/SKILL.md create mode 100644 build-logic/AGENTS.md create mode 100644 grails-gradle/AGENTS.md diff --git a/.agents/skills/build-logic-developer/SKILL.md b/.agents/skills/build-logic-developer/SKILL.md new file mode 100644 index 00000000000..2840761fc83 --- /dev/null +++ b/.agents/skills/build-logic-developer/SKILL.md @@ -0,0 +1,149 @@ +--- +name: build-logic-developer +description: Guide for working in build-logic (build-logic-root, containing :build-logic and :grails-docs-core) — the Gradle convention-plugin layer every other build in this repo (root, grails-forge, grails-gradle) consumes transitively via includeBuild. Implements GrailsJacocoPlugin, GrailsViolationAggregationPlugin, GrailsCodeStylePlugin, PublishPlugin, SbomPlugin, and 8 others, all cited by other skills without documentation until this one. Use this when changing code style/coverage/publish/SBOM/compile conventions, or anything under build-logic/. +license: Apache-2.0 +paths: build-logic/** +--- + + +## What I Do + +- Provide repository-specific guidance for `build-logic` — the Gradle convention-plugin layer, not application or framework code. Every convention referenced by other skills in this repo (violation-fixer's `aggregateViolations`, the `Coverage` section in root `AGENTS.md`, the Hibernate5/7 JaCoCo class-name-collision exclusion cited in `hibernate-developer`) is *implemented* here. +- Guide changes to the 13 registered convention plugins and their two test patterns (Gradle TestKit for task-graph behavior, plain `ProjectBuilder` for pure-logic methods). +- Flag known coverage gaps: only 4 of 13 plugins have dedicated specs. + +## When to Use Me + +Activate this skill instead of `grails-developer` when working on: + +- Anything under `build-logic/**`. +- Code style, code analysis, coverage aggregation, publishing, SBOM generation, or vulnerability-scan conventions used across the whole monorepo. +- A change to how any of the 13 convention plugins registers tasks or extensions. + +## Module Context: A Third Pattern, Not Either Existing Template + +`build-logic` has its own `settings.gradle` (like `grails-forge`), but it is **not independently versioned** — it explicitly reaches across the filesystem into root's `dependencies.gradle`/`gradle.properties` by relative path (`../../dependencies.gradle` from `plugins/build.gradle` and `docs-core/build.gradle`), the same `SharedPropertyPlugin` walk-up-the-tree mechanism `grails-gradle` uses. Neither `mongodb-developer` (fully root-governed) nor `micronaut-developer`/`grails-forge` (fully independent) is a clean fit — see `grails-gradle-developer`'s "hybrid" framing, same situation here. + +- `build-logic/gradle.properties` defines **no version properties**, only Gradle daemon/cache flags. +- No `sourceCompatibility`/`targetCompatibility`/toolchain config anywhere — compiles with whatever JDK/Groovy runs the Gradle daemon itself (Gradle 9.6.0, matching `.sdkmanrc`/root `gradle.properties`). `plugins/` applies `groovy-gradle-plugin` (Gradle's embedded Groovy, not the app-level 5.0.x); `docs-core/` explicitly depends on `"org.apache.groovy:groovy:${GroovySystem.version}"`. +- **Zero `@GrailsCompileStatic` anywhere** — all 19 plugin/extension classes use plain `@CompileStatic` (some also `@CompileDynamic` for dynamic Groovy/XML-map parsing, e.g. `GrailsViolationAggregationPlugin.groovy`). This is genuinely not Grails artefact code. +- **Zero `jakarta.*` anywhere.** `javax.inject.Inject` appears in 7 files, but it's Gradle's own DSL constructor-injection mechanism (`@Inject ObjectFactory`/`ExecOperations`, required by Gradle's plugin API, which still uses `javax.inject`), not an application-level jakarta/javax choice — don't conflate the two. +- **No dedicated CI job** — this is expected, not a dormancy signal (unlike `grails-data-neo4j`). It's exercised transitively: root, `grails-gradle`, and `grails-forge` all `includeBuild('../build-logic')`, and root's own CI jobs invoke tasks (`aggregateViolations`, `jacocoAggregateReport`, `codeStyle`) that these plugins implement. Check git log recency and README accuracy for real staleness signals instead — this module received a substantive commit (`f2c1244436`, GROOVY-12146 workaround) the day before this skill was written. + +## Module Structure + +Directory names don't match Gradle project names: + +| Directory | Gradle project | Role | +|---|---|---| +| `plugins/` | `:build-logic` | The 13 convention `Plugin` implementations (see catalog below) | +| `docs-core/` | `:grails-docs-core` | The Grails user-guide generation engine (gdoc/asciidoc → HTML/PDF), BOM-extraction tooling for docs, the version-picker dropdown generator. **README is a one-line stub** (`## grails-docs`, no body) despite containing a nontrivial BOM-extraction subsystem — a real documentation gap, not just an example. | + +## Convention Plugin Catalog + +All registered in `plugins/build.gradle` under `gradlePlugin { plugins { ... } }`. This table exists because other skills already cite several of these classes without documenting what they actually do — that's the gap this skill closes. + +| Plugin ID | Class | What it actually wires up | +|---|---|---| +| `org.apache.grails.buildsrc.compile` | `CompilePlugin` | Pins `JavaCompile.options.release`; enables sources/javadoc jars; sets manifest attrs, `duplicatesStrategy = FAIL`; applies the GROOVY-12146 reproducible-build workaround (`gradle/groovy-compile-configscript.groovy` — sorts annotation members alphabetically, since Groovy's copy-from-precompiled-class handling orders them by `Class.getDeclaredMethods()`, which varies between JVM runs); pins Javadoc/archive timestamps for reproducibility. | +| `org.apache.grails.buildsrc.publish` | `PublishPlugin` | Maven-publish + ASF-policy plumbing: `GrailsPublishExtension`, the full ASF developers/contributors POM list, Gradle Module Metadata version mapping, SHA512 checksums, fallback `LICENSE`/`NOTICE` injection, disables GPG signing when `TEST_BUILD_REPRODUCIBLE` is set. | +| `org.apache.grails.buildsrc.sbom` | `SbomPlugin` | Per-module `CyclonedxDirectTask` (deliberately *not* the full `CyclonedxPlugin`, to avoid an unwanted aggregate SBOM and Spring Boot 4's own `CycloneDxPluginAction` colliding with it), curated `LICENSE_MAPPING`/`LICENSE_EXCEPTIONS` for dependencies CycloneDX mis-detects, byte-reproducible JSON output, embeds `META-INF/sbom.json`. | +| `org.apache.grails.gradle.grails-code-style` | `GrailsCodeStylePlugin` | Applies Checkstyle+CodeNarc; materializes bundled default rule files on first access; registers `codeStyle` and `codenarcFix` (a **hand-rolled regex auto-fixer** for exactly 6 named violation types — not a real CodeNarc API, brittle by construction, covered by its own spec). | +| `org.apache.grails.gradle.grails-code-analysis` | `GrailsCodeAnalysisPlugin` | Applies PMD/SpotBugs, both **opt-in** (`grails.code-analysis.enabled.{pmd,spotbugs}` properties — returns early otherwise); SpotBugs `effort=MAX`, `reportLevel=HIGH`. | +| `org.apache.grails.gradle.grails-jacoco` | `GrailsJacocoPlugin` | Applies `JacocoPlugin`, `finalizedBy('jacocoTestReport')` on every `Test`; lazily registers the root `jacocoAggregateReport` task; **excludes Hibernate 7-suffixed subprojects' source/class dirs from the aggregate** because H7 support classes share fully-qualified names with H5 (`if (!project.path.contains('hibernate7')) { ... }`) — exec data is still included, so H7 test coverage attributes to H5's class definitions in the aggregate. | +| `org.apache.grails.gradle.grails-violation-aggregation` | `GrailsViolationAggregationPlugin` | **Root-project-only** (throws otherwise). Registers `aggregateStyleViolations`, `aggregateAnalysisViolations`, `aggregateJacocoCoverage` (with its own, separately-configured Hibernate7 exclusion prefix list, `-Pgrails.jacoco.aggregation.excludedClassPrefixes`), and umbrella `aggregateViolations`. Parses tool XML with a hardened `XmlSlurper` (external-entity/DOCTYPE disabled). This is what produces the `*_VIOLATIONS.md`/`JACOCO_COVERAGE.md` files documented in `violation-fixer`. | +| `org.apache.grails.gradle.grails-ij-formatter` | `GrailsIJFormatterPlugin` | Root-only: `installGitHooks`. All projects: `formatCode`, shells out to IntelliJ's headless CLI formatter against `.idea/codeStyles/Project.xml`, using an isolated IDE instance so it can run alongside an already-open IntelliJ. | +| `org.apache.grails.buildsrc.groovydoc-enhancer` | `GroovydocEnhancerPlugin` | Generic Groovydoc task defaults; optional direct-Ant-taskdef execution path; throws if a published module has no source dirs — "every published module must produce a groovydoc jar for Maven Central." | +| `org.apache.grails.buildsrc.groovydoc` | `GrailsGroovydocPlugin` | Grails-specific wrapper around the enhancer above — injects a hardcoded Matomo analytics footer (ASF's `analytics.apache.org`, siteId 79) into every generated Groovydoc page. | +| `org.apache.grails.buildsrc.repo` | `GrailsRepoSettingsPlugin` | `Plugin` (applied from `settings.gradle`, not a project build script) — centralizes repository lists, sets `repositoriesMode = FAIL_ON_PROJECT_REPOS`. | +| `org.apache.grails.buildsrc.properties` | `SharedPropertyPlugin` | Walks parent directories up to the ASF root (detected via `.asf.yaml`, not `.git` — `.git`-related dirs are purged from source releases) loading every `gradle.properties` found, plus `local.properties` overrides. Must be applied before any other property lookup in a composite sub-build. | +| `org.apache.grails.buildsrc.dependency-validator` | `GrailsDependencyValidatorPlugin` | Registers `validateDependencyVersions`. Auto-detects which BOM a project uses by scanning configurations (excluding `documentation`, which pulls in `grails-bom` purely for groovydoc tooling versions), fails the build if a transitive dependency silently upgraded past what the BOM pins. Opt-out via `project.ext.allowedBomOverrides`. | +| `org.apache.grails.buildsrc.vulnerability-scan` | `VulnerabilityScanPlugin` | Applies Sonatype OSS Index scanning; a shared `BuildService` (`OssIndexAuditThrottle`, `maxParallelUsages=1`) serializes `ossIndexAudit` tasks repo-wide, because the plugin's on-disk cache is file-lock guarded and throws `OverlappingFileLockException` under Gradle's default parallel execution; maintains a hand-curated CVE `excludeCoordinates` allowlist, each entry documenting the CVE and its removal condition. | + +## Testing Patterns + +Two distinct, both real — pick based on what you're testing: + +### Pattern A — Gradle TestKit (`GradleRunner`), for task-graph/execution behavior + +```groovy +class GrailsJacocoPluginSpec extends Specification { + @TempDir + Path testProjectDir + + def setup() { + testProjectDir.resolve('settings.gradle').toFile().text = '' + testProjectDir.resolve('build.gradle').toFile().text = """ + plugins { + id 'groovy' + id 'org.apache.grails.gradle.grails-jacoco' + } + repositories { mavenCentral() } + """ + } + + def "jacocoAggregateReport is registered on the root project in a multi-project build"() { + when: + def result = GradleRunner.create() + .withProjectDir(testProjectDir.toFile()) + .withArguments('tasks', '--group=verification') + .withPluginClasspath() + .build() + then: + result.output.contains('jacocoAggregateReport') + } +} +``` +Builds a real, temporary Gradle project (`@TempDir`) with a synthetic `settings.gradle`/`build.gradle`, applies the plugin by its published ID via `withPluginClasspath()`, asserts on real task graph/output. + +### Pattern B — Plain Spock + `ProjectBuilder`, for pure/static logic + +```groovy +private static Project rootWithBoms() { + Project root = ProjectBuilder.builder().withName('root').build() + ProjectBuilder.builder().withName('grails-bom').withParent(root).build() + ProjectBuilder.builder().withName('grails-micronaut-bom').withParent(root).build() + root +} + +void "detectBomPath ignores the documentation configuration when a variant BOM is used elsewhere"() { + given: + Project root = rootWithBoms() + Project project = ProjectBuilder.builder().withName('grails-micronaut').withParent(root).build() + addBomPlatform(project, 'api', ':grails-micronaut-bom') + addBomPlatform(project, 'documentation', ':grails-bom') + expect: + GrailsDependencyValidatorPlugin.detectBomPath(project) == ':grails-micronaut-bom' +} +``` +No real Gradle process — construct an in-memory project tree with `ProjectBuilder`, call the static logic directly. Faster; use it when the thing under test doesn't need real task execution. + +**Coverage gap, worth knowing before you assume a plugin is tested:** only 4 of 13 registered plugins have dedicated specs (`GrailsCodeStylePlugin`, `GrailsDependencyValidatorPlugin`, `GrailsJacocoPlugin`, `GrailsViolationAggregationPlugin`). `PublishPlugin`, `SbomPlugin`, `CompilePlugin`, `VulnerabilityScanPlugin`, `GrailsCodeAnalysisPlugin`, `GrailsIJFormatterPlugin`, the two Groovydoc plugins, `GrailsRepoSettingsPlugin`, and `SharedPropertyPlugin` have none under `plugins/src/test/groovy/`. If you touch one of the untested plugins, that's a real gap you're inheriting, not a false negative to ignore. + +## Pitfalls to Avoid + +- Do not add `@GrailsCompileStatic` anywhere in this module — see Module Context. +- Do not remove the Hibernate7 class-exclusion logic in `GrailsJacocoPlugin`/`GrailsViolationAggregationPlugin` without understanding why it exists — H5/H7 support classes share fully-qualified names, and JaCoCo cannot aggregate two different classes with the same name. +- Do not touch `gradle/groovy-compile-configscript.groovy` (referenced from `CompilePlugin`) without understanding GROOVY-12146 — it exists specifically to make annotation-member ordering deterministic across JVM runs for reproducible builds. This is recent (added 2026-07-10); check `etc/bin/normalize-annotations.groovy` and `etc/bin/verify-reproducible.sh` (added in the same commit) before assuming it's dead weight. +- Do not assume `GradleUtils.findRootGrailsCoreDir`/`findAsfRootDir` can walk via `.git` — it deliberately uses `.asf.yaml` as the root marker, because `.git`-related directories are purged from ASF source releases. `SharedPropertyPlugin`, `PublishPlugin`'s license/NOTICE fallback, and `CompilePlugin`'s config-script lookup all depend on this. +- Do not remove the `org.spockframework` exclusion when depending on `grails-gradle-model` from another module — it leaks transitively through `grails-gradle-bom`. +- If you touch `VulnerabilityScanPlugin`'s CVE `excludeCoordinates` allowlist, each entry documents *why* and *when to remove it* — don't add a bare exclusion without the same discipline. + +## Build & Test + +```bash +cd build-logic +./gradlew build # builds both :build-logic and :grails-docs-core +./gradlew :build-logic:test # plugin Spock/TestKit specs +./gradlew :grails-docs-core:test # docs-core Spock specs +./gradlew projects # confirms the two-project layout (build-logic-root -> :build-logic, :grails-docs-core) +``` +Root `AGENTS.md` documents `cd build-logic && ../gradlew build` (the *root's* wrapper, not build-logic's own `./gradlew`) — both wrappers pin the same Gradle 9.6.0 and either works, but that's the form root `AGENTS.md` uses. + +## Source of Truth + +This skill is the repository guidance for `build-logic` work. When a convention plugin's behavior changes, update this skill directly so agents load current rules from `.agents/skills/build-logic-developer/SKILL.md` — and check whether other skills that cite these classes (`hibernate-developer`'s Hibernate7 exclusion mention, root `AGENTS.md`'s `Coverage` section, `violation-fixer`) need updating too, since this is the module they're all quietly depending on. diff --git a/.agents/skills/grails-gradle-developer/SKILL.md b/.agents/skills/grails-gradle-developer/SKILL.md new file mode 100644 index 00000000000..2449f901a49 --- /dev/null +++ b/.agents/skills/grails-gradle-developer/SKILL.md @@ -0,0 +1,126 @@ +--- +name: grails-gradle-developer +description: Guide for working in grails-gradle (grails-gradle-bom, -common, -model, -plugins, -tasks) — the Gradle plugins a Grails application applies (org.apache.grails.gradle.grails-app etc.), tested via Gradle TestKit, not Spock/GORM mocking. A hybrid module — own settings.gradle and test harness, but shares dependency versions with root. Use this when changing code or tests under grails-gradle. +license: Apache-2.0 +paths: grails-gradle/** +--- + + +## What I Do + +- Provide repository-specific guidance for `grails-gradle` — the Gradle plugins that turn a plain Gradle project into a "Grails app" project (`org.apache.grails.gradle.grails-app`, `grails-web`, `grails-plugin`, etc.), not the framework runtime itself. +- Explain the hybrid nature of this module: independent `settings.gradle` and test harness, but *not* an independent version/BOM story like `grails-forge` — it deliberately shares root's `dependencies.gradle`/`gradle.properties`. +- Guide changes around the `Plugin` implementations in `plugins/`, and their Gradle TestKit tests. + +## When to Use Me + +Activate this skill instead of `grails-developer` when working on: + +- Anything under `grails-gradle/**`. +- A Gradle plugin ID → implementation class mapping (adding/changing a plugin an app's `build.gradle` applies). +- TestKit-based functional tests for a Gradle plugin. + +## Module Context: A Hybrid, Not a Clean Fit for Either Template + +`grails-gradle` is neither fully root-governed (like `grails-data-hibernate7`/`grails-data-mongodb`) nor fully independent (like `grails-forge`): + +**Shares with root:** +- Dependency *versions* — `grails-gradle/build.gradle` does `allprojects { apply from: rootProject.layout.projectDirectory.file('../dependencies.gradle') }`. Same single source of truth (`gradleBomDependencyVersions`, `bomDependencyVersions`, etc.) the main build uses. +- `javaVersion`/`projectVersion` and other root `gradle.properties` keys — `grails-gradle/gradle.properties` itself defines *no* version properties at all, only Gradle daemon/cache flags. Values reach it via `SharedPropertyPlugin` (`org.apache.grails.buildsrc.properties`, in `build-logic`), whose class doc says exactly why it exists: *"Gradle can't share properties across buildSrc or composite projects."* It walks up from `grails-gradle/` to the repo root loading every `gradle.properties` found, filling in any key not already set. +- The entire `build-logic` convention-plugin set (code style, jacoco, sbom, publish) via `includeBuild('../build-logic')` — see the `build-logic-developer` skill. + +**Diverges from root:** +- Its own `settings.gradle`, own local `gradle/test-config.gradle` (measurably different from root's — no cache-disable-in-CI block, injects `projectVersion`/`currentJdk` system properties TestKit specs depend on). +- Its own `grails-gradle-bom` (a `java-platform`, distinct from the app-facing `grails-bom`, though populated from the same `dependencies.gradle`). +- **Compiles its own module source against Gradle's embedded Groovy 4.0.32**, not root's Groovy 5.0.x (`groovy-gradle-plugin`; explicit comment in every subproject's `build.gradle`: *"compile with the Groovy version provided by Gradle"*). Verified at runtime: `cd grails-gradle && ./gradlew -v` reports Groovy 4.0.32. +- Testing: Gradle TestKit (`GradleRunner`), not root's Spock/GORM-mock conventions (see Testing Rules). + +## Where Root AGENTS.md Rules Don't Apply + +- **`@GrailsCompileStatic` is never used on grails-gradle's own classes** — plain `groovy.transform.CompileStatic` throughout (47 files). Every `@GrailsCompileStatic` string that appears in this module's source is inside javadoc/comments *describing the feature the plugin implements for a consumer app* (e.g. `GrailsCompileStaticOptions.groovy`'s "Lazy opt-ins for compiling Grails artefacts with `@GrailsCompileStatic`"), never a real annotation on a grails-gradle class. Don't "fix" this — it's build-tooling code, not a Grails artefact. +- **`javax.*` usage is legitimate here, not stale migration debt.** `javax.inject.Inject` (JSR-330 DI, unrelated to the Jakarta EE migration) and `javax.xml.parsers.*`/`javax.xml.XMLConstants` (permanent JDK APIs, never had a jakarta equivalent) both appear and are correct. The `jakarta.*` references that do exist in this module (e.g. `jakarta.servlet:jakarta.servlet-api:6.0.0` in `GroovyPagePlugin`) are about *configuring a downstream Grails application's* dependencies, not this module's own runtime. +- **Dependency versions are NOT independently pinned** (unlike `grails-forge`) — don't add a hardcoded version here; it should come from root's `dependencies.gradle` the same way the rest of the module does. + +## Module Structure + +Five Gradle projects (directory name ≠ Gradle project name in every case — check before assuming): + +| Directory | Gradle project | Role | +|---|---|---| +| `bom/` | `:grails-gradle-bom` | `java-platform` only, no source — constrains sibling subprojects, re-exports spring-boot/groovy/spock BOMs | +| `common/` | `:grails-gradle-common` | Tiny — one class, `PropertyFileUtils.groovy` | +| `model/` | `:grails-gradle-model` | Vendored/shared subset of core Grails runtime classes needed at build time before the framework is on the classpath: `grails.util.{BuildSettings,Environment,Metadata}`, `org.grails.io.support.*` resource loading, Gradle Tooling API model classes (`GrailsClasspath`) | +| `plugins/` | `:grails-gradle-plugins` | The actual `Plugin` implementations — see table below. By far the largest subproject. | +| `tasks/` | `:grails-gradle-tasks` | Only 2 files: `FindMainClassTask.groovy`, `SourceSets.groovy` | + +`plugins` depends on `common`, `tasks`, and `model` — with an explicit `exclude group: 'org.spockframework'` on the `model` dependency in both `plugins/build.gradle` and `tasks/build.gradle`, because "spock is leaking from the grails-gradle-bom through grails-gradle-model" (real comment, not a hypothetical). + +## Plugin ID → Implementation Class + +The actual entry points an app's `build.gradle` applies: + +| Plugin ID | Class | +|---|---| +| `org.apache.grails.gradle.grails-app` | `org.grails.gradle.plugin.core.GrailsGradlePlugin` | +| `org.apache.grails.gradle.grails-gsp` | `org.grails.gradle.plugin.views.gsp.GroovyPagePlugin` | +| `org.apache.grails.gradle.grails-gson` | `org.grails.gradle.plugin.views.json.GrailsGsonViewsPlugin` | +| `org.apache.grails.gradle.grails-markup` | `org.grails.gradle.plugin.views.markup.GrailsMarkupViewsPlugin` | +| `org.apache.grails.gradle.grails-plugin` | `org.grails.gradle.plugin.core.GrailsPluginGradlePlugin` (extends `GrailsGradlePlugin`) | +| `org.apache.grails.gradle.grails-profile` | `org.grails.gradle.plugin.profiles.GrailsProfileGradlePlugin` | +| `org.apache.grails.gradle.grails-web` | `org.grails.gradle.plugin.web.GrailsWebGradlePlugin` (extends `GrailsGradlePlugin`) | +| `org.apache.grails.gradle.grails-publish-profile` | `org.grails.gradle.plugin.profiles.GrailsProfilePublishGradlePlugin` | +| `org.apache.grails.gradle.grails-exploded` | `org.grails.gradle.plugin.exploded.GrailsExplodedPlugin` | +| `org.apache.grails.gradle.grails-test-phases` | `org.grails.gradle.plugin.core.TestPhasesGradlePlugin` | +| `org.apache.grails.gradle.grails-integration-test` | `org.grails.gradle.plugin.core.IntegrationTestGradlePlugin` | +| `org.apache.grails.gradle.bom-property-overrides` | `org.grails.gradle.plugin.bom.BomPropertyOverridesPlugin` | + +`GrailsGradlePlugin.apply(Project)` is the real entry point that turns a plain Gradle project into a "Grails app" project: applies core `GroovyPlugin`, enforces "cannot be both a Grails application and a Grails plugin" via a marker extension, resets `grails.util.Environment` per invocation. + +## Testing Rules + +Gradle TestKit (`GradleRunner`) is the dominant pattern in `plugins/` — **not** Spock/GORM-mock conventions from the rest of the repo. + +- **`GradleSpecification`** (`plugins/src/test/groovy/org/grails/gradle/plugin/core/GradleSpecification.groovy`) is the abstract Spock base for functional tests, documented as adapted from `apache/grails-gradle-publish`'s own `GradleSpecification`. It sets up `GradleRunner.create().withPluginClasspath().withTestKitDir(...)`, copies a fixture project from `src/test/resources/test-projects//`, does `__CURRENT_JDK__`/`__PROJECT_VERSION__` token substitution, and exposes `executeTask(String taskName, ...)`. + ```groovy + class GrailsGradlePluginJavaCompatSpec extends GradleSpecification { + def "Java 24 toolchain adds both native-access and sun-misc-unsafe-memory-access args"() { + given: + setupTestResourceProject('java-compat-toolchain-24') + when: + def result = executeTask('inspectCompatArgs') + then: + result.output.contains('HAS_NATIVE_ACCESS=true') + result.output.contains('HAS_UNSAFE_ACCESS=true') + } + } + ``` + `__CURRENT_JDK__`/`__PROJECT_VERSION__` come from JVM system properties injected specifically by `grails-gradle/gradle/test-config.gradle`'s `systemProperty 'projectVersion', ...` / `systemProperty 'currentJdk', ...` — this is exactly why grails-gradle's local `test-config.gradle` differs from root's. +- **28 real fixture project directories** under `plugins/src/test/resources/test-projects/` (e.g. `java-compat-toolchain-24`, `bom-platform-hibernate7-micronaut-auto`) — each a minimal, real Gradle project TestKit builds and inspects. Add a new fixture directory here for a new functional test, don't try to synthesize a project inline. +- Non-TestKit unit specs also exist for pure-unit-testable classes with no `Project` interaction (e.g. `BomManagedVersionsSpec`, `GrailsCompileStaticOptionsSpec`) — plain `spock.lang.Specification`, no `GradleRunner`. Use this lighter pattern when the logic under test doesn't touch a real Gradle build/task graph. + +## Pitfalls to Avoid + +- Do not add `@GrailsCompileStatic` to grails-gradle's own classes — see "Where Root AGENTS.md Rules Don't Apply." +- Do not hardcode a dependency version here — pull it from root's `dependencies.gradle` the way the rest of the module does; don't create a third, module-local version source. +- Do not assume a fixture-project functional test can be synthesized inline — the `GradleSpecification` pattern expects a real directory under `test-projects/`. +- Remember `plugins` depends on `model` with `org.spockframework` explicitly excluded — don't remove that exclusion without understanding it leaks transitively through `grails-gradle-bom`. + +## Build & Test + +```bash +cd grails-gradle +./gradlew build --continue --stacktrace +./gradlew build -PskipTests -PskipCodeStyle # fast build +./gradlew validateDependencyVersions --continue --stacktrace # BOM validation, run separately from root's own +./gradlew aggregateStyleViolations --continue # CodeNarc/Checkstyle +./gradlew jacocoAggregateReport --continue --stacktrace -PskipCodeStyle # coverage +``` +All verified against actual CI invocations (`.github/workflows/{gradle,codestyle,coverage,codeanalysis,release}.yml`, all `working-directory: grails-gradle`). + +## Source of Truth + +This skill is the repository guidance for `grails-gradle` work. When module conventions change, update this skill directly so agents load current rules from `.agents/skills/grails-gradle-developer/SKILL.md`. diff --git a/AGENTS.md b/AGENTS.md index 181ffef28cb..e360c3eaf5b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,11 +87,11 @@ This repository contains multiple independent Gradle projects: | Project | Description | Build Command | |---------|-------------|---------------| | **grails-core** (root) | Main framework with 60+ modules | `./gradlew build` | -| **build-logic/** | Gradle convention plugins for the build | `cd build-logic && ../gradlew build` | -| **grails-gradle/** | Grails Gradle plugins | `cd grails-gradle && ./gradlew build` | +| **build-logic/** | Gradle convention plugins for the build — every other build here consumes it transitively; see [`build-logic/AGENTS.md`](build-logic/AGENTS.md) | `cd build-logic && ../gradlew build` | +| **grails-gradle/** | Grails Gradle plugins — hybrid: own settings.gradle/tests, but shares root's dependency versions; see [`grails-gradle/AGENTS.md`](grails-gradle/AGENTS.md) | `cd grails-gradle && ./gradlew build` | | **grails-forge/** | Application generator (like Spring Initializr) — Micronaut, not Grails; see [`grails-forge/AGENTS.md`](grails-forge/AGENTS.md) | `cd grails-forge && ./gradlew build` | -Each project has its own `settings.gradle` and independent build. When working on a specific project, run Gradle commands from that project's directory. `grails-forge/` has its own nested `AGENTS.md` — read it before working there, since most of this file's Grails/GORM/Hibernate content doesn't apply to it. +Each project has its own `settings.gradle` and independent build. When working on a specific project, run Gradle commands from that project's directory. All three subprojects above have their own nested `AGENTS.md` — read the relevant one before working there. `grails-forge/` is the cleanest case (fully independent, most of this file's Grails/GORM/Hibernate content doesn't apply); `grails-gradle/` and `build-logic/` are hybrids — independent `settings.gradle`/testing, but they deliberately share this file's dependency-version management rather than pinning their own. ## Dependency Management diff --git a/build-logic/AGENTS.md b/build-logic/AGENTS.md new file mode 100644 index 00000000000..a27a9dd5a00 --- /dev/null +++ b/build-logic/AGENTS.md @@ -0,0 +1,71 @@ + + +# Agent Guide for build-logic + +> **IMPORTANT**: `build-logic` is the Gradle convention-plugin layer every other build in this +> repo (root, [`grails-forge`](../grails-forge/AGENTS.md), [`grails-gradle`](../grails-gradle/AGENTS.md)) +> consumes transitively via `includeBuild`. It has its own `settings.gradle`, but — like +> `grails-gradle`, unlike `grails-forge` — it is NOT independently versioned: it reaches across +> the filesystem into root's `dependencies.gradle`/`gradle.properties` by relative path. Read this +> file for what's actually different; root [`AGENTS.md`](../AGENTS.md) still governs PR/branch/review +> conventions and repository-wide policy. + +## Quick Reference + +Run from inside `build-logic/`, not the repo root: + +```bash +./gradlew build # builds both :build-logic and :grails-docs-core +./gradlew :build-logic:test # plugin Spock/TestKit specs +./gradlew :grails-docs-core:test # docs-core Spock specs +``` +Root `AGENTS.md` documents `cd build-logic && ../gradlew build` — the root's own wrapper, not this directory's `./gradlew`. Both pin Gradle 9.6.0; either works, but that's the documented form. + +## Critical Rules (corrected for this subproject) + +Checked against actual source: + +1. **Use plain `@CompileStatic`, NOT `@GrailsCompileStatic`.** Zero real usages of the latter anywhere in this module — it's build-tooling code, not a Grails artefact. All 19 plugin/extension classes use plain `@CompileStatic`. +2. **No `jakarta.*`, and the `javax.*` present is legitimate.** `javax.inject.Inject` appears 7 times — it's Gradle's own DSL constructor-injection mechanism (required by Gradle's plugin API, which still uses `javax.inject`), unrelated to the Jakarta EE migration. Don't "fix" it. +3. **Dependency versions are NOT independently pinned.** `build-logic/gradle.properties` has no version properties at all — `plugins/build.gradle` and `docs-core/build.gradle` both explicitly `apply from: '../../dependencies.gradle'` (root's file) and read root's `gradle.properties` directly. Don't hardcode a version here. +4. **No dedicated CI job is expected, not a staleness signal.** Every other build in this repo `includeBuild('../build-logic')` and exercises these plugins transitively through their own CI runs. Don't assume "no CI job" means dormant the way it did for `grails-data-neo4j` — check git log recency and README accuracy directly instead. +5. **Apache license header, 4 spaces, no tabs** — unchanged from root. + +## Available Skills + +Same directory-based discovery as root: list `../.agents/skills/*/SKILL.md`, read each front-matter `description`, load the ones that match. The skill most relevant to this subproject is `build-logic-developer`, which catalogs all 13 registered convention plugins (what each actually configures, not just its name) and both testing patterns used here. + +```bash +for f in ../.agents/skills/*/SKILL.md; do awk -F': *' '/^description:/{print FILENAME": "$2; exit}' "$f"; done +``` + +## Project Structure + +| Directory | Gradle project | Role | +|---|---|---| +| `plugins/` | `:build-logic` | The 13 convention-plugin implementations | +| `docs-core/` | `:grails-docs-core` | Grails user-guide generation engine + BOM-extraction tooling for docs (README is a one-line stub — don't trust it to reflect what's actually here) | + +See the `build-logic-developer` skill for the full plugin catalog and testing patterns — this table is orientation only. + +## Why This Matters More Than Its Size Suggests + +Several other skills in this repo already cite classes implemented here without documenting them: `hibernate-developer`'s note on Hibernate5/7 JaCoCo class-name collisions, root `AGENTS.md`'s `Coverage` section (`aggregateJacocoCoverage`, `jacocoAggregateReport`), and `violation-fixer`'s `*_VIOLATIONS.md`/`JACOCO_COVERAGE.md` output. If you change a convention plugin's behavior here, check whether those other documents need updating too — this module is the one they're all quietly depending on. + +## What's Unchanged From Root AGENTS.md + +PR guidelines, branch naming, review process, adversarial self-review, security reporting, and the single-large-PR-over-reviewability-stack default all apply here exactly as documented in [`../AGENTS.md`](../AGENTS.md) — this file doesn't repeat them so they can't drift out of sync with the root copy. Read that file for anything not covered above. diff --git a/grails-gradle/AGENTS.md b/grails-gradle/AGENTS.md new file mode 100644 index 00000000000..7b7ef56f27b --- /dev/null +++ b/grails-gradle/AGENTS.md @@ -0,0 +1,75 @@ + + +# Agent Guide for grails-gradle + +> **IMPORTANT**: `grails-gradle` is a **hybrid** subproject — unlike [`grails-forge`](../grails-forge/AGENTS.md), +> it is NOT independent of the root build's dependency management. It has its own `settings.gradle` +> and its own test harness (Gradle TestKit, not Spock/GORM mocking), but it deliberately shares +> `dependencies.gradle`/`gradle.properties` with the repo root via `SharedPropertyPlugin`. Read this +> file for what's actually different; root [`AGENTS.md`](../AGENTS.md) still governs PR/branch/review +> conventions, dependency *versions*, and repository-wide policy. + +## Quick Reference + +Run from inside `grails-gradle/`, not the repo root: + +```bash +./gradlew build --continue --stacktrace +./gradlew build -PskipTests -PskipCodeStyle +./gradlew validateDependencyVersions --continue --stacktrace +./gradlew aggregateStyleViolations --continue +./gradlew jacocoAggregateReport --continue --stacktrace -PskipCodeStyle +``` + +## Critical Rules (corrected for this subproject) + +Checked against actual source, not assumed: + +1. **Use plain `@CompileStatic`, NOT `@GrailsCompileStatic`.** This inverts the root rule. Grails-gradle is build tooling — Gradle plugins, not Grails artefacts. Every `@GrailsCompileStatic` string in this module's own source is javadoc describing the feature the plugin implements *for a consumer app*, never a real annotation here. Confirmed: 47 files use plain `@CompileStatic`, zero use `@GrailsCompileStatic` as an actual annotation. +2. **`javax.*` is legitimate here, not stale migration debt.** `javax.inject.Inject` (JSR-330, unrelated to Jakarta EE) and `javax.xml.*` (permanent JDK APIs) both appear correctly. Don't "fix" these to `jakarta.*` — they were never part of that migration. The `jakarta.*` references that do exist here are about configuring a *downstream Grails application's* dependencies, not this module's own runtime. +3. **Dependency versions are NOT independent — this is the opposite of `grails-forge`.** `grails-gradle/gradle.properties` defines no version properties at all, only Gradle daemon/cache flags. Versions come from root's `dependencies.gradle` (via `allprojects { apply from: '../dependencies.gradle' }`) and root's `gradle.properties` (via `SharedPropertyPlugin`, which walks up the directory tree loading every `gradle.properties` it finds). Don't hardcode a version here; don't assume `validateDependencyVersions` doesn't apply — it does, just run separately (see Quick Reference). +4. **Module source compiles against Gradle's embedded Groovy 4.0.32, not root's Groovy 5.0.x.** This is a real, structural split (`groovy-gradle-plugin`), not a version-drift bug. Don't "upgrade" it to match root — it can't, by design, since it compiles inside Gradle's own plugin classpath. +5. **Testing is Gradle TestKit (`GradleRunner`), not Spock/GORM-mock conventions.** See the `grails-gradle-developer` skill for the actual pattern (`GradleSpecification` base class, fixture projects under `test-projects/`). +6. **Apache license header, 4 spaces, no tabs** — unchanged from root. + +## Available Skills + +Same directory-based discovery as root: list `../.agents/skills/*/SKILL.md`, read each front-matter `description`, load the ones that match. The skill most relevant to this subproject is `grails-gradle-developer`, which covers the plugin-ID-to-class mapping, TestKit testing patterns, and the shared-vs-divergent build split in depth. + +```bash +for f in ../.agents/skills/*/SKILL.md; do awk -F': *' '/^description:/{print FILENAME": "$2; exit}' "$f"; done +``` + +## Project Structure + +| Directory | Gradle project | Role | +|---|---|---| +| `bom/` | `:grails-gradle-bom` | `java-platform` only — constrains sibling subprojects | +| `common/` | `:grails-gradle-common` | Tiny — one shared-utility class | +| `model/` | `:grails-gradle-model` | Vendored subset of Grails runtime classes needed at build time before the framework is on the classpath | +| `plugins/` | `:grails-gradle-plugins` | The actual `Plugin` implementations — see `grails-gradle-developer` skill for the ID→class table | +| `tasks/` | `:grails-gradle-tasks` | Two small task classes | + +See the `grails-gradle-developer` skill for code patterns and the full plugin catalog — this table is orientation only. + +## CI + +`.github/workflows/gradle.yml`, `codestyle.yml`, `coverage.yml`, `codeanalysis.yml`, and `release.yml` all have dedicated `working-directory: grails-gradle` jobs — this subproject is actively CI'd independently of root, unlike some other independent subprojects in this repo. + +## What's Unchanged From Root AGENTS.md + +PR guidelines, branch naming, review process, adversarial self-review, security reporting, and the single-large-PR-over-reviewability-stack default all apply here exactly as documented in [`../AGENTS.md`](../AGENTS.md) — this file doesn't repeat them so they can't drift out of sync with the root copy. Read that file for anything not covered above. From b0c2454e29d7f94fede162b414b8d96770981def Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 21:44:26 -0500 Subject: [PATCH 17/20] Link grails-developer to grails-test-examples/ ~50 real functional test apps live under grails-test-examples// (hibernate7, mongodb, micronaut, spring-security, geb, etc.) - each a real, standalone Grails app exercising a specific framework feature - and nothing pointed grails-developer (the app-development skill) at them. Real risk: several share a name with a framework-internals module (grails-test-examples/hibernate7/ vs grails-data-hibernate7/), so without an explicit signal an agent could reach for the wrong skill's mental model in the wrong place. Adds paths: grails-test-examples/** (additive, not restrictive - this is still a repo-wide skill per its own description) plus a short section grounding the skill's generic app-layout guidance in this repo's actual location, and the real -PonlyFunctionalTests/ -PskipFunctionalTests gating flags (verified wired into 9 gradle config files, not just README claims). --- .agents/skills/grails-developer/SKILL.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.agents/skills/grails-developer/SKILL.md b/.agents/skills/grails-developer/SKILL.md index a6856c4e75c..b42cd1d832d 100644 --- a/.agents/skills/grails-developer/SKILL.md +++ b/.agents/skills/grails-developer/SKILL.md @@ -1,7 +1,8 @@ --- name: grails-developer -description: Comprehensive guide for current Grails development, covering web applications, REST APIs, GORM, controllers, services, views, plugins, and testing with Spock and Geb +description: Comprehensive guide for current Grails development, covering web applications, REST APIs, GORM, controllers, services, views, plugins, and testing with Spock and Geb. In this repo, grails-test-examples/ holds ~50 real functional test apps (one per framework feature — hibernate7, mongodb, micronaut, spring-security, geb, etc.) that this skill's app-development guidance applies to directly, as opposed to the framework-internals skills (hibernate-developer, mongodb-developer, etc.). license: Apache-2.0 +paths: grails-test-examples/** --- +--- +name: migration-scoping +description: Before starting any refactor/rewrite/optimization-shaped task on a core subsystem (GORM registry, datastore internals, binder/mapping layer, etc.), classify it as mechanical (bounded, safe to just do) or architectural (a project, not a patch) and check whether another local or remote branch already attempted it. Use this before writing code for anything that sounds like "improve/refactor/optimize/rewrite X", not for ordinary bug fixes or additive features. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +metadata: + audience: maintainers + frameworks: grails +--- + +## What I Do + +- Classify an incoming task as mechanical (bounded, single-session, no design decisions) or architectural (changes core behavior/object model, has more than one reasonable design) before any code gets written. +- For architectural work specifically, check local and remote branches for prior art on the same subsystem before starting, so a new attempt doesn't silently duplicate or conflict with one already in flight or already abandoned. +- Report findings — this skill doesn't decide *which* prior attempt to build on, that's a design call for the user; it surfaces what exists so the call can be made with full information instead of by accident. + +## Why This Exists + +Doing mechanical fixes first and discovering an architectural blocker last is wasted effort — a general lesson from legacy-modernization work (a rewrite of a removed dependency, say, needs to be identified and scoped as its own project *before* any mechanical patching begins around it, not discovered halfway through). This failure mode showed up concretely in this repo: four branches independently attempted the same GORM registry/scaling work — `8.0.x-hibernate7`, `8.0.x-hibernate7.gorm-registry-refactor`, `8.0.x-hibernate7.gorm-scaling-clean`, and `fix/gorm-api-registration-scaling` — before anyone checked whether prior art existed. Ancestry analysis (`git merge-base --is-ancestor`) showed `gorm-registry-refactor` and the 456-file `gorm-scaling-clean` rewrite were both fully orphaned dead ends (never merged anywhere, not ancestors of each other), while `fix/gorm-api-registration-scaling` was the branch that had actually landed and was already baked into the live chain that became `feat/neo4j-gorm-registry-migration`. That determination took real archaeological work *after the fact* — this skill exists so it happens *before*, when it's cheap. + +## When to Use Me + +Activate before writing any code for a task shaped like: + +- "Improve/refactor/optimize/rewrite [core subsystem]" +- "Make [GORM registry / datastore internals / binder / mapping layer] faster/cleaner/scale better" +- Any request that touches `grails-datastore-core`, `grails-datamapping-core`, or a datastore module's mapping-context/binder/registry internals in a way that isn't a narrow, obviously-bounded fix + +**Not needed for:** ordinary bug fixes, additive features with a single clear implementation, style/violation cleanup, dependency bumps, documentation. + +## Step 1: Classify + +Ask: does this change alter core behavior or the object model in a way that has more than one reasonable design, or touch a subsystem multiple other pieces of code depend on? + +- **Mechanical** — bounded, one clear way to do it, completable in the current session (a renamed API, a deployment-target bump, a straightforward bug fix). Proceed normally, no further triage needed. +- **Architectural** — a rewrite of how a subsystem works, a new abstraction layer, a scaling/performance redesign, anything with real design-space to explore. This is a project, not a patch — go to Step 2 before writing any code. + +If unsure which one it is, treat it as architectural. The cost of over-triaging a mechanical task is a few minutes of `git log`; the cost of under-triaging an architectural one is the four-branch scenario above. + +## Step 2: Check for Prior Art (architectural work only) + +```bash +# What's the actual current work on this subsystem? Look for branches/PRs whose name or +# recent commits mention the subsystem you're about to touch. +git branch -a | grep -i '' +gh pr list --repo apache/grails-core --search ' in:title' --state all + +# For any candidate found, determine its real relationship to the current branch/mainline — +# do NOT trust branch names or dates alone (see Pitfalls below). +git merge-base +git merge-base --is-ancestor && echo "already merged/subsumed" +git merge-base --is-ancestor origin/ && echo "landed on mainline" +git log --oneline .. | wc -l # how much unique work is actually there +git diff --stat # how large/real is that work +``` + +Classify each candidate found: + +- **Landed** (ancestor of mainline or the branch you're about to build on) — its work is already yours, don't redo it. +- **Live** (has an open PR, or is clearly the branch other recent work descends from) — coordinate instead of duplicating; surface this to the user before proceeding. +- **Orphaned** (not an ancestor of anything, no open PR, no recent activity) — a prior attempt that didn't land. Worth reading before you start (it may show a design that was tried and abandoned for a reason, or simply ran out of steam) but don't silently build on it as if it were current. + +## Pitfalls (from the real four-branch case) + +- **Branch names lie.** `8.0.x-hibernate7.gorm-registry-refactor` sounds like it should be the predecessor of `8.0.x-hibernate7.gorm-scaling-clean` — it wasn't; they were siblings, and neither was an ancestor of the other. Don't infer lineage from naming alone; check with `git merge-base --is-ancestor`. +- **Last-commit date is a weak, sometimes-inverted signal.** In the real case, the branch with the *older* last-commit timestamp (`fix/gorm-api-registration-scaling`) was the one that actually won and landed — the newer-looking branches were dead ends. Ancestry, not recency, is the real signal. +- **Large diffs don't imply "more complete" or "more current."** The 456-file `gorm-scaling-clean` rewrite was the most extensive of the four branches and also the most thoroughly abandoned one — size is not a proxy for correctness or currency. +- **A merged-somewhere-else branch can still block deletion of a worktree/branch that isn't actually dead.** Cross-check against the [`worktree-hygiene`](../worktree-hygiene/SKILL.md) skill's PR-state check before assuming "orphaned" — a branch with no local merge but a live remote PR is not dead, it's in review. + +## Source of Truth + +This skill is the repository guidance for pre-work architectural triage. If the actual worked example above (the four Hibernate7/GORM-registry branches) gets pruned from git history entirely, keep the pattern and the pitfalls — they're general, not specific to that one incident. From e234b831d4f974d1e8ff148e15d7323007b76156 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sat, 11 Jul 2026 23:08:13 -0500 Subject: [PATCH 20/20] Address Copilot review comments on PR #15977 - Fix stale AGENTS.md example: grails-developer now has paths:, use groovy-developer/java-developer as the no-paths example instead; clarify compatibility: is optional/unverified-if-absent. - Stop hardcoding Gradle's embedded Groovy patch version in grails-gradle/AGENTS.md; point at gradle-wrapper.properties instead so it doesn't drift on wrapper bumps. - Move front-matter above the license comment block in four new SKILL.md files (worktree-hygiene, migration-scoping, micronaut-developer, diff-coverage-check) to match the repo's existing convention (front-matter first, e.g. violation-fixer). Co-Authored-By: Claude Sonnet 5 --- .agents/skills/diff-coverage-check/SKILL.md | 18 +++++++++--------- .agents/skills/micronaut-developer/SKILL.md | 20 ++++++++++---------- .agents/skills/migration-scoping/SKILL.md | 18 +++++++++--------- .agents/skills/worktree-hygiene/SKILL.md | 20 ++++++++++---------- AGENTS.md | 4 ++-- grails-gradle/AGENTS.md | 2 +- 6 files changed, 41 insertions(+), 41 deletions(-) diff --git a/.agents/skills/diff-coverage-check/SKILL.md b/.agents/skills/diff-coverage-check/SKILL.md index a8db03279eb..d4fa0dc6d74 100644 --- a/.agents/skills/diff-coverage-check/SKILL.md +++ b/.agents/skills/diff-coverage-check/SKILL.md @@ -1,3 +1,12 @@ +--- +name: diff-coverage-check +description: Computes real diff coverage (coverage of only the lines you actually changed, not whole-file coverage) entirely locally, without CI or Codecov — by running each affected module's own tests and cross-referencing its JaCoCo XML against git diff. Use before committing, or when asked "is my change covered" / "check coverage on the files I touched" / "diff coverage". A change often spans several modules; run this per module. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +metadata: + audience: maintainers + frameworks: grails +--- ---- -name: diff-coverage-check -description: Computes real diff coverage (coverage of only the lines you actually changed, not whole-file coverage) entirely locally, without CI or Codecov — by running each affected module's own tests and cross-referencing its JaCoCo XML against git diff. Use before committing, or when asked "is my change covered" / "check coverage on the files I touched" / "diff coverage". A change often spans several modules; run this per module. -license: Apache-2.0 -compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf -metadata: - audience: maintainers - frameworks: grails ---- ## What I Do diff --git a/.agents/skills/micronaut-developer/SKILL.md b/.agents/skills/micronaut-developer/SKILL.md index 1a6792db732..107d13a03ac 100644 --- a/.agents/skills/micronaut-developer/SKILL.md +++ b/.agents/skills/micronaut-developer/SKILL.md @@ -1,3 +1,13 @@ +--- +name: micronaut-developer +description: Guide for working in grails-forge (grails-forge-core, grails-forge-api, grails-forge-cli, grails-forge-web-netty) — a Micronaut application, not a Grails one. Covers Micronaut DI/bean patterns, HTTP controllers, two distinct Spock testing patterns (MicronautTest for HTTP controllers, ApplicationContextSpec for Feature classes), Picocli CLI commands, Rocker templating, and the Feature extension-point system. Use this instead of grails-developer/hibernate-developer when changing code under grails-forge/. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +paths: grails-forge/** +metadata: + audience: maintainers + frameworks: micronaut +--- ---- -name: micronaut-developer -description: Guide for working in grails-forge (grails-forge-core, grails-forge-api, grails-forge-cli, grails-forge-web-netty) — a Micronaut application, not a Grails one. Covers Micronaut DI/bean patterns, HTTP controllers, two distinct Spock testing patterns (MicronautTest for HTTP controllers, ApplicationContextSpec for Feature classes), Picocli CLI commands, Rocker templating, and the Feature extension-point system. Use this instead of grails-developer/hibernate-developer when changing code under grails-forge/. -license: Apache-2.0 -compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf -paths: grails-forge/** -metadata: - audience: maintainers - frameworks: micronaut ---- ## What I Do diff --git a/.agents/skills/migration-scoping/SKILL.md b/.agents/skills/migration-scoping/SKILL.md index f57d8b09e13..9ee9ae8d762 100644 --- a/.agents/skills/migration-scoping/SKILL.md +++ b/.agents/skills/migration-scoping/SKILL.md @@ -1,3 +1,12 @@ +--- +name: migration-scoping +description: Before starting any refactor/rewrite/optimization-shaped task on a core subsystem (GORM registry, datastore internals, binder/mapping layer, etc.), classify it as mechanical (bounded, safe to just do) or architectural (a project, not a patch) and check whether another local or remote branch already attempted it. Use this before writing code for anything that sounds like "improve/refactor/optimize/rewrite X", not for ordinary bug fixes or additive features. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +metadata: + audience: maintainers + frameworks: grails +--- ---- -name: migration-scoping -description: Before starting any refactor/rewrite/optimization-shaped task on a core subsystem (GORM registry, datastore internals, binder/mapping layer, etc.), classify it as mechanical (bounded, safe to just do) or architectural (a project, not a patch) and check whether another local or remote branch already attempted it. Use this before writing code for anything that sounds like "improve/refactor/optimize/rewrite X", not for ordinary bug fixes or additive features. -license: Apache-2.0 -compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf -metadata: - audience: maintainers - frameworks: grails ---- ## What I Do diff --git a/.agents/skills/worktree-hygiene/SKILL.md b/.agents/skills/worktree-hygiene/SKILL.md index 6be8c968906..7a9817e70b6 100644 --- a/.agents/skills/worktree-hygiene/SKILL.md +++ b/.agents/skills/worktree-hygiene/SKILL.md @@ -1,3 +1,13 @@ +--- +name: worktree-hygiene +description: Reports stale or orphaned git worktrees under .claude/worktrees/ (the agent-managed worktree directory) by checking each worktree's branch against its GitHub PR state and merge-into-default-branch status, not commit age. Use at the start of a session in this repo when .claude/worktrees/ has accumulated entries, or when asked to clean up worktrees or check branch hygiene. Report-only — never deletes without explicit confirmation, and never touches worktrees outside .claude/worktrees/. +license: Apache-2.0 +compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf +paths: .claude/worktrees/** +metadata: + audience: maintainers + frameworks: grails +--- ---- -name: worktree-hygiene -description: Reports stale or orphaned git worktrees under .claude/worktrees/ (the agent-managed worktree directory) by checking each worktree's branch against its GitHub PR state and merge-into-default-branch status, not commit age. Use at the start of a session in this repo when .claude/worktrees/ has accumulated entries, or when asked to clean up worktrees or check branch hygiene. Report-only — never deletes without explicit confirmation, and never touches worktrees outside .claude/worktrees/. -license: Apache-2.0 -compatibility: opencode, claude, grok, gemini, copilot, cursor, windsurf -paths: .claude/worktrees/** -metadata: - audience: maintainers - frameworks: grails ---- ## What I Do diff --git a/AGENTS.md b/AGENTS.md index cb9297c7bb8..a339f39aad0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,9 +64,9 @@ export GRADLE_OPTS="-Xms2G -Xmx5G" > for f in .agents/skills/*/SKILL.md; do awk -F': *' '/^description:/{d=$2} /^paths:/{p=$2} END{print FILENAME": "d (p?" [paths: "p"]":"")}' "$f"; done > ``` > -> Some skills also declare an optional front-matter `paths:` glob (e.g. `paths: grails-data-hibernate7/**`) scoping them to a specific module — if the file(s) you're touching match a skill's `paths`, load it regardless of whether you'd have matched it on description alone. `paths` is a stronger, structural signal than prose; not every skill needs one (repo-wide skills like `grails-developer`/`groovy-developer` intentionally have none). +> Some skills also declare an optional front-matter `paths:` glob (e.g. `paths: grails-data-hibernate7/**`) scoping them to a specific module — if the file(s) you're touching match a skill's `paths`, load it regardless of whether you'd have matched it on description alone. `paths` is a stronger, structural signal than prose; not every skill needs one (repo-wide skills like `groovy-developer`/`java-developer` intentionally have none). Skills may also declare an optional `compatibility:` field listing which agent tools they've been verified against — its absence doesn't mean a skill is unusable, just unverified. > -> The directory is the source of truth, not a list in this file — a hardcoded skill index here would drift the moment a skill is added, renamed, or removed. Each `SKILL.md`'s front-matter (`name`, `description`, `paths`, `compatibility`) is what makes it discoverable to any agent, per the Agent Skills Specification. +> The directory is the source of truth, not a list in this file — a hardcoded skill index here would drift the moment a skill is added, renamed, or removed. Each `SKILL.md`'s front-matter (`name`, `description`, and optionally `paths`, `compatibility`) is what makes it discoverable to any agent, per the Agent Skills Specification. ## Technology Stack diff --git a/grails-gradle/AGENTS.md b/grails-gradle/AGENTS.md index 7b7ef56f27b..249da9fde33 100644 --- a/grails-gradle/AGENTS.md +++ b/grails-gradle/AGENTS.md @@ -42,7 +42,7 @@ Checked against actual source, not assumed: 1. **Use plain `@CompileStatic`, NOT `@GrailsCompileStatic`.** This inverts the root rule. Grails-gradle is build tooling — Gradle plugins, not Grails artefacts. Every `@GrailsCompileStatic` string in this module's own source is javadoc describing the feature the plugin implements *for a consumer app*, never a real annotation here. Confirmed: 47 files use plain `@CompileStatic`, zero use `@GrailsCompileStatic` as an actual annotation. 2. **`javax.*` is legitimate here, not stale migration debt.** `javax.inject.Inject` (JSR-330, unrelated to Jakarta EE) and `javax.xml.*` (permanent JDK APIs) both appear correctly. Don't "fix" these to `jakarta.*` — they were never part of that migration. The `jakarta.*` references that do exist here are about configuring a *downstream Grails application's* dependencies, not this module's own runtime. 3. **Dependency versions are NOT independent — this is the opposite of `grails-forge`.** `grails-gradle/gradle.properties` defines no version properties at all, only Gradle daemon/cache flags. Versions come from root's `dependencies.gradle` (via `allprojects { apply from: '../dependencies.gradle' }`) and root's `gradle.properties` (via `SharedPropertyPlugin`, which walks up the directory tree loading every `gradle.properties` it finds). Don't hardcode a version here; don't assume `validateDependencyVersions` doesn't apply — it does, just run separately (see Quick Reference). -4. **Module source compiles against Gradle's embedded Groovy 4.0.32, not root's Groovy 5.0.x.** This is a real, structural split (`groovy-gradle-plugin`), not a version-drift bug. Don't "upgrade" it to match root — it can't, by design, since it compiles inside Gradle's own plugin classpath. +4. **Module source compiles against Gradle's own embedded Groovy version, not root's Groovy 5.0.x.** This is a real, structural split (`groovy-gradle-plugin`), not a version-drift bug. Don't "upgrade" it to match root — it can't, by design, since it compiles inside Gradle's own plugin classpath. The embedded version tracks whatever Gradle version `gradle/wrapper/gradle-wrapper.properties` points at; don't hardcode a patch number here, it drifts on every wrapper bump. 5. **Testing is Gradle TestKit (`GradleRunner`), not Spock/GORM-mock conventions.** See the `grails-gradle-developer` skill for the actual pattern (`GradleSpecification` base class, fixture projects under `test-projects/`). 6. **Apache license header, 4 spaces, no tabs** — unchanged from root.