Automate repository conventions and stage static analysis enforcement

[jamesfredley]

Description

This PR turns recurring, objective review feedback into deterministic repository checks while deliberately leaving subjective engineering judgment in review.

The starting point was an audit of recent merged and proposed changes in the #16012 through #15688 PR-number window. The recurring mechanical concerns were action immutability, static-analysis drift, duplicate message keys, agent-skill metadata drift, report trustworthiness, and license/provenance checks. Repeating those comments by hand does not scale. This change creates a ratchet: enforce the clean baseline now, preserve compatibility for broader opt-in analysis, and expand enforcement only as the recorded backlog is resolved.

No Grails runtime API or application behavior changes in this PR.

What this moves forward

1. PMD becomes blocking where it is already clean

PMD is now enabled by full Gradle project path for the four modules that produce clean, non-empty PMD XML today:

• :grails-data-graphql-core
• :grails-data-mongodb-spring-data
• :grails-datasource
• :grails-testing-support-core

The implementation supports both staged project allowlists and the existing global opt-in flags. It also preserves grails.code-analysis.ignoreFailures=true for report-only baseline runs.

Analysis and style reports are now tied to exact task-owned outputs instead of scanning a broad directory. Before analyzers run, active outputs are cleaned. Each analyzer that actually executes writes a relocatable marker containing the configured XML output's root-relative path, so custom directories and filenames remain associated without serializing an absolute machine path. A genuinely missing report always fails while an unscheduled or legitimate NO-SOURCE task is omitted. One writer task owns each pair of Markdown summaries, runs after scheduled analyzers, and is reached through a no-output finalizer even when an analyzer fails. Direct single-module analyzer tasks therefore remain usable, consumers cannot race a second report writer, and build-cache relocation remains portable. Marker filenames encode the full project path, so nested projects with the same leaf name cannot collide.

Generated files below each project's configured build directory are excluded from PMD inputs without excluding normal sources when a checkout ancestor happens to be named build.

2. Repository conventions have one public Gradle surface

./gradlew validateRepositoryConventions now validates the canonical repository root and writes build/reports/violations/REPOSITORY_CONVENTIONS.md.

It checks these scenarios:

• Canonical .agents/skills/*/SKILL.md files start with valid top-level YAML front matter containing string name, description, and license values.
• Skill names match their directory and are unique.
• Every canonical skill path is listed in AGENTS.md, and every listed path exists.
• Workflow and local action-manifest YAML is parsed with SnakeYAML's data-only SafeConstructor.
• Every external uses reference is pinned to a lowercase 40-character commit SHA.
• A given external action uses one consistent SHA across workflows and composite actions.
• Docker uses, Docker action runs.image, and workflow job/service container images use immutable lowercase sha256 digests rather than mutable tags.
• Duplicate YAML keys and malformed YAML fail closed.
• Logical keys in messages*.properties are unique, including escaped separators and continued lines.
• Generated/build trees do not create false duplicate-key findings.
• RAT runs before repository conventions so license provenance remains part of the same developer-facing gate.

The task is wired into aggregateStyleViolations and aggregateViolations only for the canonical root. Independent builds such as grails-gradle retain their existing aggregate style behavior and do not require a local AGENTS.md.

3. GitHub Actions are immutable by construction

Remaining symbolic external action references were replaced with verified commit SHAs, including Checkout, Cache, Setup Java, and the shared Grails action repository. The Mongo service image is pinned to its Docker Hub manifest digest rather than the mutable mongo:8 tag.

The new validator covers ordinary steps, reusable workflow jobs, quoted keys, inline YAML maps, YAML aliases, and repository-local composite actions in any action.yml or action.yaml location, including a path segment named build. Local ./... references remain valid, but any external actions used inside their manifests are still validated.

4. Release provenance matches RAT provenance

Agent-local state, configuration symlinks, and guidance files excluded from RAT are now excluded from the source ZIP with matching patterns. The source checkout no longer persists GitHub credentials, and Info-ZIP stores any remaining repository symlinks as links rather than dereferencing their targets. This prevents an excluded file or linked external path from being shipped unaudited.

5. Existing deterministic drift is repaired

• Removed the duplicate spring.security.ui.forgotPassword.email.line4 message key.
• Corrected stale agent-skill guidance and added the canonical mono-repository integration skill to AGENTS.md.
• Updated the violation-fixer skill with the staged-analysis properties, truthful disabled-tool report semantics, and the new repository convention workflow.

Why the rollout is staged

Enabling every analyzer everywhere in this PR would require thousands of unrelated source edits and would make the convention work impossible to review safely. This PR goes as far as the repository can go without mixing a broad cleanup campaign into the build-policy change:

• It makes currently clean, report-producing PMD modules blocking.
• It keeps global PMD and SpotBugs baseline modes available.
• It preserves repository-wide SpotBugs execution in separate non-blocking CI advisory passes with distinct artifacts while Stabilize SpotBugs task wiring and detector execution #16014 tracks task-wiring and detector failures.
• It reports disabled tools explicitly instead of calling them clean.
• It files quantified follow-up issues for every known area that cannot be enabled safely yet.
• It adds the infrastructure needed to extend the allowlists one module at a time.

Subjective concerns such as architecture, compatibility judgment, semantic documentation quality, test sufficiency, and quote/type preferences remain review-only. They are documented for agents but are not encoded as brittle mechanical rules.

Remaining work and follow-up issues

Issue	Remaining work	Why it is not in this PR
#16014	Stabilize SpotBugs task wiring and detector execution	Broad runs currently expose undeclared generated-output dependencies and detector IndexOutOfBoundsException failures. Tool execution must be reliable before enforcement.
#16018	Triage the provisional 287-finding SpotBugs baseline	The baseline must be regenerated after #16014, then fixed or narrowly documented before modules can become blocking.
#16017	Resolve 5,551 PMD findings across the remaining root modules	Fixing these findings would create a large, unrelated source diff. The issue records maintained-source counts and the incremental allowlist process.
#16016	Resolve 113 PMD findings in grails-gradle-model	The independent Gradle-plugin build remains report-only until its existing model findings are fixed.
#16015	Resolve 224 maintained-source PMD findings in Grails Forge and add supported staged CI enforcement	Forge intentionally has no aggregate analysis task, so its explicit source-bearing PMD task set has a separate cleanup and enforcement path.

The clean PMD baseline was corrected during verification: grails-scaffolding, grails-spring-security, and grails-spring-security-acl initially appeared as zero-finding modules, but their PMD tasks produce no XML because they have no PMD-analyzable Java source. They are not presented as clean and are not allowlisted.

Suggested next steps

1. Resolve Stabilize SpotBugs task wiring and detector execution #16014 so SpotBugs completes without task-wiring or detector failures.
2. Regenerate and triage the SpotBugs baseline in Triage the existing SpotBugs finding baseline #16018, then introduce a staged SpotBugs project allowlist.
3. Start PMD cleanup with the smallest root modules in Enable PMD incrementally across remaining core modules #16017 and add each report-producing clean module to the blocking allowlist in the same PR.
4. Complete the focused grails-gradle-model cleanup in Resolve existing PMD findings in grails-gradle-model #16016.
5. Clean Forge's 224 maintained-source findings from the smallest modules upward, then make its explicit source-bearing PMD task set blocking in Forge CI under Scope and resolve PMD findings in grails-forge #16015.

Verification

• build-logic: ..\gradlew.bat build --no-watch-fs --no-daemon
• Independent grails-gradle: .\gradlew.bat aggregateStyleViolations --no-watch-fs --no-daemon --max-workers=2
• Independent grails-gradle PMD baseline: .\gradlew.bat aggregateAnalysisViolations --continue "-Pgrails.code-analysis.enabled.pmd=true" "-Pgrails.code-analysis.ignoreFailures=true" --rerun-tasks --no-watch-fs --no-daemon --max-workers=2; it reports only the 113 maintained-source findings in :grails-gradle-model and omits three legitimate NO-SOURCE tasks.
• Direct analyzer surface: .\gradlew.bat :grails-datasource:pmdMain --no-watch-fs --no-daemon --max-workers=2; the single requested analyzer and its finalizer succeed without false missing reports for other allowlisted modules.
• Root selective analysis: .\gradlew.bat aggregateAnalysisViolations --continue --no-daemon --max-workers=2 --no-watch-fs
• Code-analysis CI preserves repository-wide SpotBugs execution as separate continue-on-error advisory passes for the root and grails-gradle, each uploaded under a distinct artifact name after the enforced PMD reports.
• Root report-only baseline: .\gradlew.bat aggregateAnalysisViolations --continue "-Pgrails.code-analysis.enabled.pmd=true" "-Pgrails.code-analysis.ignoreFailures=true" --no-watch-fs --no-daemon --max-workers=2; 5,551 maintained-source findings across 53 source-bearing modules.
• Mandatory pre-commit gate: .\gradlew.bat clean aggregateViolations :grails-test-report:check --continue --no-daemon --no-parallel --max-workers=2 --no-watch-fs
• The mandatory run completed 1,984 tasks and generated the full test-report matrix. All convention, style, selective-analysis, and report aggregation tasks passed. The aggregate integration report recorded 3,617 tests, 2 failures, 0 errors, and 98 skipped. One unchanged Windows-only mail example failed because its GSP renders CRLF while the test hard-codes LF; a serial targeted run reproduced the same Hello\r\nWorld! versus Hello\nWorld! mismatch in MailServiceSpec. The unchanged SiteMesh async multiple-layout scenario received a transient HTTP 500 during the full run; an isolated serial rerun of EndToEndSpec then passed all 14 tests. Neither example implementation nor test is changed by this PR.
• Final root aggregateViolations after all review fixes and rebasing onto origin/8.0.x: 751 tasks completed successfully.
• Final reports: Checkstyle, CodeNarc, PMD, and repository conventions are clean; SpotBugs explicitly reports SpotBugs is disabled.
• Manual failure path: a quoted mutable action, inline Docker tag, mutable Docker action image, mutable service container, build-path local action, and repository path escape were rejected with structural YAML locations, then the fixtures were removed and the clean task rerun.
• Invalid PMD root/unknown project paths, configured build-directory exclusion without checkout-path false positives, unsuppressible missing reports, failed-analyzer report writing, stale-output removal after a later writer failure, direct single-project style/analysis, legitimate NO-SOURCE, custom report directories and filenames, portable root-relative markers, single-writer ordering, duplicate nested project names, top-level skill metadata typing, duplicate YAML keys, malformed YAML, semantic versus ordinary uses fields, recursive build-path actions, repository path escapes, case-insensitive Docker metadata, immutable container images, and configuration-cache reuse are covered by TestKit.

The Windows validation run disabled Gradle file-system watching because Gradle's native gradle-fileevents.dll crashed a long-lived daemon. The same checks passed with --no-watch-fs; no product or test failure was hidden.

Open PR #15977 also changes the AGENTS.md skill inventory. It is not a semantic duplicate of this work, but the documentation overlap should be resolved when either branch is rebased.

Contributor Checklist

Issue and Scope

• No pre-existing implementation issue covered this cross-cutting convention work. The audit background and rationale are documented above; all discovered legacy cleanup was separated into Stabilize SpotBugs task wiring and detector execution #16014 through Triage the existing SpotBugs finding baseline #16018.
• This PR completes its stated scope: deterministic enforcement infrastructure and the currently clean baseline. The linked follow-up issues are explicitly separate cleanup projects, not unfinished code in this PR.
• This PR contains one focused change: turn recurring objective repository review conventions into automated checks and a staged analysis ratchet.
• This PR targets 8.0.x, the current major-development branch.

Code Quality

• Tests were added or updated for every changed build-logic behavior.
• The separate ./gradlew build --rerun-tasks command was not run because no runtime module behavior changed. The mandatory :grails-test-report:check matrix did run: 3,617 integration tests reported the two unchanged example failures described above. A serial mail run confirmed the Windows CRLF mismatch, while an isolated SiteMesh rerun passed all 14 tests. The affected build-logic suite, independent grails-gradle surfaces, and 751-task root aggregate gate pass.
• The project code-style and aggregate violation workflows pass.
• This PR does not contain mass reformatting or broad legacy-analysis cleanup.
• Generative tooling used a quality model and the final diff was independently reviewed for goal fit, code quality, security, hands-on QA, and repository context.

Licensing and Attribution

• New source files contain Apache License 2.0 headers, and RAT passes.
• The contribution is original work prepared for this repository.
• Generative tooling use follows ASF policy and is attributed in the commit metadata.

Documentation

• Developer-facing guidance in AGENTS.md and the violation-fixer skill documents the new commands, properties, reports, and remediation paths.
• This build-policy change does not add a Grails user-facing feature, so no What's New entry is required.
• This change does not alter runtime APIs or require application upgrade steps, so no Upgrade Notes entry is required.
• The description explains the problem, each enforced scenario, the staged boundary, verification, remaining issues, and next steps.

[Copilot]

Pull request overview

This PR adds deterministic build-time enforcement for previously manual repository hygiene checks (repository conventions, immutable GitHub Actions/Docker pins, message key uniqueness) and introduces staged, allowlist-based static analysis enforcement (PMD now blocking only where clean), without changing Grails runtime behavior.

Changes:

• Introduces validateRepositoryConventions (RAT + skills/AGENTS/workflow/action YAML + container pins + messages key uniqueness) and wires it into aggregation tasks for the canonical root.
• Refactors violation aggregation to use task-owned report outputs via root-relative marker files, cleaning scheduled outputs and failing on genuinely missing reports.
• Stages static-analysis enforcement via per-project PMD/SpotBugs allowlists and updates CI workflows + agent guidance accordingly.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
grails-spring-security/ui/plugin/grails-app/i18n/messages.spring-security-ui.properties	Removes a duplicate message key entry to satisfy uniqueness validation.
gradle/rat-root-config.gradle	Aligns RAT excludes with agent-local state/config files so provenance checks remain consistent.
gradle.properties	Adds the staged PMD enforcement allowlist for currently clean modules.
build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/RepositoryConventionsTaskSpec.groovy	Adds TestKit coverage for repository conventions validation scenarios.
build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPluginSpec.groovy	Expands TestKit coverage for marker-based aggregation, allowlists, and disabled-tool reporting.
build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/GrailsCodeStylePluginSpec.groovy	Verifies code style plugin also registers code analysis surface.
build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/GrailsCodeAnalysisPluginSpec.groovy	Adds tests for allowlists and build-dir exclusions in PMD configuration.
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/RepositoryConventionsTask.groovy	Implements the repository conventions validator (skills, actions, container pins, messages keys).
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy	Reworks style/analysis aggregation to use markers, staged allowlists, and deterministic report lifecycle.
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeStylePlugin.groovy	Emits marker files for style reports and applies code analysis plugin alongside style.
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeAnalysisPlugin.groovy	Adds per-project allowlists, marker emission, and build-dir source exclusion for PMD.
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GradleUtils.groovy	Adds project-path encoding for collision-free report naming + marker helpers.
build-logic/plugins/build.gradle	Adds SnakeYAML dependency to support safe YAML parsing in repository conventions checks.
AGENTS.md	Documents validateRepositoryConventions and updates skill inventory/guidance for staged analysis behavior.
.github/workflows/vulnerability-scan.yml	Pins external actions to immutable SHAs.
.github/workflows/release.yml	Pins reusable actions to SHAs, disables checkout credential persistence, and aligns ZIP excludes with RAT.
.github/workflows/release-publish-docs.yml	Pins reusable actions to immutable SHAs.
.github/workflows/release-close.yml	Pins reusable actions to immutable SHAs.
.github/workflows/groovy-joint-workflow.yml	Pins Mongo service image to an immutable digest.
.github/workflows/gradle.yml	Pins cache and deploy actions to immutable SHAs.
.github/workflows/codeanalysis.yml	Switches to staged enforcement + adds SpotBugs advisory passes with distinct artifacts.
.agents/skills/violation-fixer/SKILL.md	Updates guidance to reflect staged analysis and new conventions task/report semantics.
.agents/skills/mono-repo-integration/SKILL.md	Moves skill YAML front matter to the top so it conforms to the new validator.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.4507%. Comparing base (dfebb9a) to head (8bc7b8f).
⚠️ Report is 7 commits behind head on 8.0.x.

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #16025        +/-   ##
==================================================
+ Coverage     51.4404%   51.4507%   +0.0103%     
- Complexity      17728      17731         +3     
==================================================
  Files            2039       2039                
  Lines           95497      95507        +10     
  Branches        16564      16564                
==================================================
+ Hits            49124      49139        +15     
+ Misses          39074      39067         -7     
- Partials         7299       7301         +2     

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jamesfredley]

@jdaugherty @matrei @borinquenkid @sbglasius this plus the newly created issues, should get us closer to PRs automatically following the expected syntax and passing all of the automated checkers. This PR was the next logical step.

Should save us immense time on PR reviews, once we get this gradually in place.

[borinquenkid]

@jdaugherty @matrei @borinquenkid @sbglasius this plus the newly created issues, should get us closer to PRs automatically following the expected syntax and passing all of the automated checkers. This PR was the next logical step.

Should save us immense time on PR reviews, once we get this gradually in place.

This is a good beginning. In a follow up start adding SKILL.md for automating the clean up of some of these issues. The CodeNarc script is broken.

[jdaugherty]

I'd like to review this before merging, but am currently traveling. I can take a look at this by Friday.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 8bc7b8f
▶️ Tests: 12795 executed
⚪️ Checks: 59/59 completed

---

Learn more about TestLens at testlens.app.

[jamesfredley]

@jdaugherty no rush and you will find this somewhat incomplete, with several things carved out to issues. I am sure it needs some polish somewhere.

@borinquenkid Agreed, this adds two skills.md files, but more will be better.