Implement the apply goal as a Maven-native port of prevent-overwrites.sh

[Jardo-51]

Initial implementation of com.jardoapps:pao-maven-plugin:0.1.0 — a Maven-native port of the prevent-artifact-overwrites shell script.

Approach

A single apply goal, run as its own invocation before the publishing build:

mvn -B com.jardoapps:pao-maven-plugin:0.1.0:apply
mvn -B deploy

Two invocations are a constraint, not a preference: Maven reads and interpolates every POM before any mojo executes, so a version written during a build does not change what that same build deploys. Running as a lifecycle extension would allow a single invocation, but it would then execute on every developer mvn call and every IDE import — the compile-time version rewriting the original project deliberately avoids.

No POM changes are required. Every environment variable of the shell version maps to a pao.* property, and .prevent-overwrites.conf is supported unchanged, so the existing CI interface carries over by swapping one line.

ApplyMojo is parameter plumbing only; the behaviour lives in PreventOverwritesRunner, which works on plain types and can be driven directly from tests without the Maven testing harness.

Rather than depending on versions-maven-plugin internals, PomDocument scans a pom.xml into elements carrying their source offsets and splices edits into the original text. Formatting, comments and attribute quoting survive untouched, and comments/CDATA/quoted attributes are skipped so a <version> inside a comment is never rewritten.

Bugs in the shell version this fixes

Each was reproduced against prevent-overwrites.sh before being fixed here.

1. Multi-module reactors break. enforce_branch_version only rewrites a <version> outside <parent>, never a module's <parent><version>. On a two-module project the child kept 1.2.3-SNAPSHOT while the aggregator moved, and mvn validate then failed with Non-resolvable parent POM. The plugin moves parent references across the reactor in step with the project version.
2. Two-digit patch numbers are truncated. The prefix pattern matches a single patch digit, so 1.2.10-feature-x-SNAPSHOT strips to 1.2.1-SNAPSHOT — a silently wrong version.
3. Pin validation was working around sed. Pinned values were restricted to a safe character set only because they flowed into a sed replacement. With structured editing the check is now the actual requirement: that a pin is revertible.

A fourth was caught by these tests during development — an ambiguous -rc pattern that read 1.2.3-rc.4-SNAPSHOT as base 1.2.3 plus suffix rc.4, which would have dropped the qualifier on a core branch. Fixed with possessive quantifiers.

Intentional behaviour changes

Both documented in the README:

• Property-indirected versions are followed. <version>${revision}</version> updates the <revision> property rather than replacing the reference. Undefined properties are left alone with a warning.
• Every config row is validated, not just rows matching the current branch, so a typo fails on the first run instead of lying dormant until that branch is next built.

Testing

• 71 unit tests. The twelve cases from the shell test suite are ported and compare output to the original fixture files byte for byte, which is what pins down formatting preservation.
• 5 integration tests (bash it/run-integration-tests.sh) cover what only a real Maven invocation exercises: parameter binding, reactor collection from the session, and git operations against real repositories. The round-trip test asserts mvn validate still resolves after the version change — the check that fails on the shell version.

Not included

Maven Central distribution (signing, staging, a release profile) is not set up, so the plugin currently resolves only from a local mvn install or an internal repository.

🤖 Generated with Claude Code

[Jardo-51]

Code review summary

Severity	Count
CRITICAL	1
HIGH	1
MEDIUM	5
LOW	6

Overall assessment: needs rework

The design is clean and the splice-based POM editing is a genuinely good call - formatting, comments and ${property} indirection all survive, and the test suite (71 tests, green locally) is thorough for the paths it covers. The blocker is a gap in the enforce path: when the project version changes, <dependency> entries pointing at other reactor modules are not moved with it, so a multi-module project ends up building against the shared x.y.z-SNAPSHOT from the repository - precisely the artifact this plugin exists to protect. Below that, the 120s git timeout in CommandLineGitClient can never fire, and git commit -a commits more than the plugin changed.

General notes

• No CI workflow. The repository has no .github/ directory, so neither mvn test nor it/run-integration-tests.sh runs on a PR, and the GitHub Actions snippet the README recommends to users is itself never exercised. Worth adding before this merges, given the plugin's whole job is to be correct inside someone else's pipeline.
• Integration coverage is thin on the paths most likely to break. it/run-integration-tests.sh covers the round trip, idempotence, env detection, an invalid pin and skip. Nothing exercises core-branch dependency stripping, ${revision} versions, or per-branch pinning against a real Maven invocation, and no IT covers the multi-module dependency case described in the CRITICAL finding.
• Documentation is accurate for what the code does. The one gap: the README's "Multi-module projects" section states the whole reactor is handled in one run and mentions only <parent><version>; whichever way the CRITICAL finding is resolved, that paragraph needs to say what happens to inter-module <dependency> versions.

Verification notes: findings 1, 5 and 8 were reproduced by driving PreventOverwritesRunner against scratch reactors; the rest are from reading. mvn -o test passes at b56dd9f.

[Jardo-51]

Verification pass over all 13 unresolved review threads, at 334ce17 (working tree clean and equal to the PR head). All 13 check out against the code and are now resolved.

Thread	Severity	Fix	Verdict
Sibling reactor dependency not moved	CRITICAL	c2208f4	Addressed
TIMEOUT_SECONDS can never fire	HIGH	6ebfe57	Addressed
git commit -a commits too much	MEDIUM	b3d24ef	Addressed
configureUser writes .git/config	MEDIUM	f3fb2e6	Addressed
Release version turned into a snapshot	MEDIUM	ed82c97	Addressed
GITHUB_REF_NAME on pull-request events	MEDIUM	7eec1cf	Addressed
POM encoding read/written as UTF-8	MEDIUM	cfc5879	Addressed
RunResult.projectVersion was stale	LOW	6a24e74	Addressed
Unconfigured vs. missing config file	LOW	fb21539	Addressed
Extra config columns silently dropped	LOW	9cf3990	Addressed
IT summary double-counts a failed test	LOW	3bd3d14	Addressed
Glob character class copied verbatim	LOW	b8e98ef	Addressed
Git identity never asserted in tests	LOW	334ce17	Addressed

Each fix was traced to the code rather than taken from the reply, and the four with the most substance were checked by mutation — reverting the fix in a scratch worktree and confirming the named tests fail: the sibling-dependency loop (2 failures in MultiModuleTest), the prolog encoding (3 failures in PomDocumentTest), the glob class body (3 failures in GlobMatcherTest), and the IT counting guard (summary drops to 6 passed, 1 failed when an assertion is forced to fail, instead of counting the test twice). The git timeout was reproduced directly against a child that holds stdout open.

Tests at 334ce17, run locally since the repository has no CI workflow: mvn -o test 100 tests, 0 failures; bash it/run-integration-tests.sh 7 passed, 0 failed.

Two General notes from the review summary have no thread and are still open:

• No CI workflow. There is still no .github/ directory, so neither suite runs on a PR and the Actions snippet the README recommends is never exercised.
• Integration coverage. test_commit_scope and test_leaves_no_git_config were added, but the IT reactor is still my-parent + core with no inter-module dependency, so the CRITICAL scenario is covered only by unit tests. ${revision} versions, a successful per-branch pin, and core-branch dependency stripping remain without integration coverage.

One thing to note rather than act on: 7eec1cf also added CI_MERGE_REQUEST_SOURCE_BRANCH_NAME for GitLab merged-results pipelines, which goes beyond the case that was reproduced. It is correct and symmetric with the GitHub handling, and the offer to pull it back out was never taken up, so it stays.