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/diff-coverage-check/SKILL.md b/.agents/skills/diff-coverage-check/SKILL.md new file mode 100644 index 00000000000..d4fa0dc6d74 --- /dev/null +++ b/.agents/skills/diff-coverage-check/SKILL.md @@ -0,0 +1,140 @@ +--- +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. + +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 + +```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. 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/** --- + +## 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/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 `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`), 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. + +## 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 HTTP Controllers: MicronautTest + Spock + +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 +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") + } +} +``` + +### 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: + +```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`. diff --git a/.agents/skills/migration-scoping/SKILL.md b/.agents/skills/migration-scoping/SKILL.md new file mode 100644 index 00000000000..9ee9ae8d762 --- /dev/null +++ b/.agents/skills/migration-scoping/SKILL.md @@ -0,0 +1,87 @@ +--- +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. diff --git a/.agents/skills/mongodb-developer/SKILL.md b/.agents/skills/mongodb-developer/SKILL.md new file mode 100644 index 00000000000..aee3ac7c4a4 --- /dev/null +++ b/.agents/skills/mongodb-developer/SKILL.md @@ -0,0 +1,122 @@ +--- +name: mongodb-developer +description: Guide for working in the grails-data-mongodb module (GORM for MongoDB) — codec-based document mapping, GeoJSON, embedded documents, TTL indexes, multi-tenancy, and Testcontainers-backed tests. Use this when changing code or tests under grails-data-mongodb. +license: Apache-2.0 +paths: grails-data-mongodb/** +--- + + +## 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`. 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/skills/worktree-hygiene/SKILL.md b/.agents/skills/worktree-hygiene/SKILL.md new file mode 100644 index 00000000000..7a9817e70b6 --- /dev/null +++ b/.agents/skills/worktree-hygiene/SKILL.md @@ -0,0 +1,99 @@ +--- +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 + +- 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. diff --git a/AGENTS.md b/AGENTS.md index 5ec9ac09512..a339f39aad0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,27 +58,15 @@ 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` -> - Indexing code -> Read `.agents/skills/codebase-memory/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:/{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 `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`, and optionally `paths`, `compatibility`) is what makes it discoverable to any agent, per the Agent Skills Specification. ## Technology Stack @@ -99,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` | -| **grails-forge/** | Application generator (like Spring Initializr) | `cd grails-forge && ./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. +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 @@ -236,6 +224,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. + +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. + ## Branch Naming (Auto-Labels PRs) | Prefix | Label | @@ -248,13 +251,33 @@ 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` +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. -6. **Squash commits** into a single meaningful commit message -6. **Reference issues** in PR description (e.g., "Fixes #1234") +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)**: Before requesting human review, get a fresh-context pass on the diff — a new agent session/conversation with no memory of writing the change, not the same context that just wrote it — and explicitly instruct it to try to *refute* the change: find what's wrong, not confirm it looks fine. That adversarial framing is what does the work, not which vendor's model runs it — a same-tool fresh-context review with a refute-first instruction catches most of what a second vendor would, and it's what every contributor actually has access to, unlike a second AI subscription. If you do have access to a different model/vendor (e.g. Codex, Gemini), prefer that for the strongest independence — but it's an upgrade, not a requirement. Check against this file's rules (jakarta not javax, no wildcard imports, BOM version rules, test coverage) and 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") ### Review Process 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-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-gradle/AGENTS.md b/grails-gradle/AGENTS.md new file mode 100644 index 00000000000..249da9fde33 --- /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 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. + +## 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.