feat: add purge-check goal to detect and remove orphaned build directories

[gnodet]

Summary

Adds a new purge-check goal to detect and remove orphaned build output directories left behind by sub-projects that have been removed from the reactor.

Problem

When a sub-project is removed from the reactor (e.g. after git pull or a branch switch), its target/ directory may remain on disk. Since the sub-project is no longer in the reactor, mvn clean cannot know about it and skips it. This causes issues with plugins that scan the project tree recursively — notably RAT which finds unlicensed build artifacts and fails the build.

See #315, apache/maven-resolver#1943, apache/maven#11800.

Solution

New goal clean:purge-check with heuristic:

A direct child directory of basedir is considered orphaned when its only non-hidden child is the build output directory (typically target/). A live sub-project always has at least a pom.xml alongside its build directory.

This avoids false positives while covering the exact scenario of a removed sub-project (which leaves only a target/ behind).

The goal uses NIO2 (DirectoryStream, Files.isDirectory), respects ${project.build.directory} for the build dir name, and reuses the existing Cleaner for deletion.

Lifecycle integration

This PR only adds the goal. The binding to the initialize phase of the default lifecycle will be done separately in Maven core 4.1.0, so it runs automatically on every build (not just mvn clean), preventing stale artifacts from interfering with RAT, Checkstyle, etc.

Users on earlier Maven versions can bind it manually:

<execution>
  <id>purge-check</id>
  <phase>initialize</phase>
  <goals><goal>purge-check</goal></goals>
</execution>

Changes

• CleanOrphansMojo — new mojo, NIO2, reuses Cleaner
• src/it/purge-check-orphan/ — IT: creates an orphaned sub-orphan/target/ via setup.groovy, verifies it is removed while sub-existing/target/ (which has siblings) is left intact

Fixes #315

[gnodet-bot]

Solid implementation. The NIO2 usage is correct (DirectoryStreams properly closed via try-with-resources), the orphan detection heuristic is conservative and well-documented, and the Cleaner API is used consistently with CleanMojo. The IT coverage is good.

Two low-severity observations below.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 9f617c6 — the new commit correctly restores the missing closing brace and Javadoc opener in CleanerTest.java. Syntax fix is accurate.

Two observations from the previous review remain outstanding (both non-blocking):

1. verify.groovy:32-34 — the triple-OR assertion is logically correct but hard to reason about; the simpler !log.readLines().any { ... } alternative was suggested
2. CleanOrphansMojo.java:171 — after cleaner.delete(buildDir), the parent directory (e.g. sub-orphan/) is left empty on disk; whether to also remove it is a scope decision

Neither blocks the PR.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Two observations — one dead code, one confirmation of gnodet-bot's readability finding.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 59acbe7 — the refactor to a single DirectoryStream per candidate directory (orphanBuildDir()) is clean and correct: resource handling is proper (try-with-resources), early return on second visible entry is efficient, and the edge cases (empty dir, non-directory sole entry, wrong name) are all handled.

Two findings from previous reviews remain outstanding:

1. Session injection (line 80) — still injected, still unused
2. verify.groovy assertion (lines 32-34) — still contains the triple-OR with dead logic

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 90e0375 — the two new commits correctly address the previous findings:

1. verify.groovy assertion — ✅ Addressed. Replaced the triple-OR with the simplified !log.find('Removing orphaned.*sub-existing') as suggested.
2. Parent directory left on disk — ✅ Addressed. The mojo now removes the entire orphaned parent directory (not just its target/ subdirectory). isOrphanDirectory() returns boolean, findOrphanBuildDirectories() collects the parent path, IT verifies !orphanDir.exists() on the whole parent. Clean refactor.

One finding from previous reviews remains outstanding:

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 70233df — all three previous findings are now addressed:

1. verify.groovy assertion — ✅ Simplified to !log.find('Removing orphaned.*sub-existing')
2. Parent directory cleanup — ✅ Mojo now removes the entire orphaned directory, not just target/
3. Unused Session injection — ✅ Removed in 70233df (both field and import)

No new issues in the incremental diff. Code is solid.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 70233df — the final outstanding finding is now addressed:

1. Session injection (dead code) — ✅ Addressed. Both the @Inject private Session session field and its import are removed. This was the last remaining item from previous reviews.

All findings from previous review cycles are now resolved. Code is clean — NIO2 resource handling is correct, orphan detection heuristic is conservative and well-tested, IT coverage validates both the positive case (orphan removed) and the negative case (active sub-project preserved).

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[elharo]

The strategy for identifying orphaned files seems sound. I'm not convinced a new goal is needed. This seems like something the existing clean goal should just do.

[gnodet]

Thank you for the feedback! I'd like to clarify the use case a bit, because the problem this goal addresses isn't really about the clean goal itself.

The issue (#315) describes a situation where a sub-module is removed or renamed from a multi-module build. After that, the old target/ directory stays on disk indefinitely — even if you run mvn clean, because clean only cleans modules that are still in the reactor. The orphaned target/ directory is effectively invisible to Maven.

The typical scenario is: git pull --rebase && mvn install. A colleague removed or renamed a sub-module upstream; after the rebase the directory is gone from the POM but the target/ is still there on disk. The next build happily picks up stale classes, reports, or generated files from it.

This causes real problems with plugins that scan the project tree by filesystem path rather than by reactor membership:

• Apache RAT recursively scans target/ and fails on binary/generated files
• Checkstyle, SpotBugs, maven-site-plugin can pick up stale classes or reports

The purge-check goal addresses this by running in the initialize phase — before any of those plugins — and removing target/ directories whose parent directory no longer has a pom.xml. Binding to initialize is important because clean is a separate lifecycle: when a user runs mvn install or mvn verify, the clean lifecycle is not invoked at all, so there is no opportunity for the clean goal to remove orphaned directories before RAT or Checkstyle get to them.

The heuristic is intentionally conservative: a directory is flagged as orphaned only if target/ is its sole visible child. A directory that still contains a pom.xml — whether it was renamed or is simply a clean module — is never touched.

One alternative we considered was hardcoding this behavior directly in maven-core — which was actually the original idea. It would be slightly faster (no plugin classloading overhead) and always present without any binding. However, embedding file deletion logic in maven-core makes it harder to discover, configure, or disable on a per-project basis. Keeping it in maven-clean-plugin makes the behavior visible in mvn help:effective-pom and the build plan, and lets projects opt out with -Dmaven.clean.purgeCheck.skip=true or override parameters the usual way.

The intent is to bind this goal into both the default and clean lifecycles in Maven 4.1.0 (apache/maven#11800), so that orphan cleanup becomes automatic for all users without any plugin configuration — similar to how maven-clean-plugin:clean is already bound to the clean phase today. This PR is the first step toward that.

Does that help clarify the rationale? Happy to adjust the implementation or the naming if you have suggestions.

[gnodet-bot]

Re-review after a5b5e68 — the new commit correctly addresses @desruisseaux's feedback:

1. Parameter-level @since 3.5.1 tags — ✅ Removed. This mojo is new and never existed in 3.5.1, so parameter-level @since tags were incorrect.
2. Class-level @since — ✅ Updated from 3.5.1 to 4.0.0-beta-3, matching the target milestone.

No logic changes, Javadoc-only fix. All previous findings remain addressed.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after CI_GREEN on a5b5e68 — CI passes. All previous bot findings remain addressed.

Two new inline comments from @desruisseaux (posted after the last bot APPROVE) are outstanding:

1. isVisible() → Files.isHidden() — the custom isVisible(Path) method (line 208) checks !name.startsWith("."), but Files.isHidden(Path) is the proper NIO2 API. It does the same thing on Unix, and additionally handles the Windows hidden-attribute — more portable.
2. Remove the isVisible() method after switching to Files.isHidden().

Both are valid suggestions. Needs one more commit before merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 221f14a — the last outstanding committer feedback is now addressed:

1. isVisible() → Files.isHidden() — ✅ Addressed. The DirectoryStream filter now uses p -> !Files.isHidden(p) instead of the custom isVisible() method. More portable (handles Windows hidden attribute in addition to Unix dot-prefix).
2. Dead isVisible() method — ✅ Removed.

All findings from all review cycles are resolved. Code is solid.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after e92d1eb — the new commit addresses @desruisseaux's suggestion to test Files.isDirectory last: the filename string comparison (sole.getFileName().toString().equals(buildDirName)) now precedes the filesystem call, avoiding an unnecessary I/O operation when the name doesn't match. Logically equivalent, strictly better.

All findings from all previous review cycles remain addressed. Code is solid.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after b6b800b — Spotless formatting only: the return statement in isOrphanDirectory() was collapsed from 3 lines to 1. No logic change. All previous findings remain addressed.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[rmannibucau]

Ok for me as a manual option but ultimately for maven 4.0.0 it must be automatic and in core since core owns that fake repo and reactor thing, not plugins IMHO

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved symlink handling, scan-error behavior, test coverage, and documentation updates block approval.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Exclude symbolic-link children from orphan scanning · New
• Continue scanning after per-child inspection failures · New

What changed in this PR

Adds clean:purge-check to detect and remove orphaned sub-project build directories using NIO2 and the existing Cleaner.

Changes:

• Implements orphan detection and cleanup.
• Adds Maven 4 integration-test fixtures and verification.
• Configures Invoker execution for the integration test.

File	Summary
src/​main/​java/​org/​apache/​maven/​plugins/​clean/​CleanOrphansMojo.java	Implements the new purge goal; symlink handling and per-child inspection errors require changes.
src/​it/​purge-check-orphan/​verify.groovy	Verifies purge behavior; should directly assert preservation of the active fixture.
src/​it/​purge-check-orphan/​sub-existing/​pom.xml	Defines the active sub-project fixture.
src/​it/​purge-check-orphan/​setup.groovy	Creates orphan and active-project test fixtures.
src/​it/​purge-check-orphan/​pom.xml	Configures the integration-test project and goal execution.
src/​it/​purge-check-orphan/​invoker.properties	Restricts execution to Maven 4.

The user-facing goal documentation also requires updating before publication.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[gnodet-bot]

Re-review — assessing new Copilot findings on b6b800b

Two new findings from Copilot arrived after the last bot APPROVE. Both are valid defensive improvements, neither is blocking:

1. Symlink handling (line 164) — Confirmed valid. Files.newDirectoryStream(basedir, Files::isDirectory) follows symlinks by default, so a symlink pointing to a directory whose only visible child is target/ would be falsely treated as orphaned. Fix: add && !Files.isSymbolicLink(child) to the filter or as a guard in isOrphanDirectory(). In practice, symlinks as Maven sub-module directories are extremely rare, but the defense-in-depth is consistent with the Cleaner's followSymlinks=false default.

2. IOException resilience (line 173) — Confirmed valid. An IOException from isOrphanDirectory() for one child aborts the entire loop. Moving the try/catch inside the loop (warn per child, continue scanning) would be more resilient. The outer catch already degrades gracefully (warns instead of failing), so the impact is limited to missing subsequent orphan detections when one child is unreadable.

Both are edge cases unlikely to trigger in normal Maven usage, but worth addressing in a follow-up.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 2cc8c28 — the new commit correctly addresses the two outstanding Copilot findings:

1. Symlink exclusion — ✅ Addressed. Both Files.isDirectory() calls now use LinkOption.NOFOLLOW_LINKS: the candidate scan filter (line 165) excludes symlinks from being considered as potential orphans, and the final check in isOrphanDirectory() (line 205) prevents treating symlink targets as build directories. Consistent with the Cleaner's followSymlinks=false default.

2. Per-child IOException resilience — ✅ Addressed. The try/catch is now inside the for-loop, so an IOException from isOrphanDirectory() on one child logs a warning and continues scanning remaining siblings. The outer catch remains for failure to open the root stream.

All findings from all previous review cycles are resolved. Code is solid.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 2cc8c28 — the two outstanding Copilot findings are now addressed:

1. Symlink handling — ✅ Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS) in both the directory stream filter and isOrphanDirectory() return. Symlinks to directories are excluded from orphan scanning, consistent with the Cleaner's followSymlinks=false default.
2. Per-child IOException resilience — ✅ try/catch moved inside the for (Path child : children) loop. An unreadable sibling no longer aborts the scan — a warning is logged and remaining children continue to be inspected.

All findings from all review cycles are resolved. Code is solid.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

Ok for me as a manual option but ultimately for maven 4.0.0 it must be automatic and in core since core owns that fake repo and reactor thing, not plugins IMHO

@rmannibucau This is not the [root]/.mvn/target directory, this is normal directories, made orphaned because if a supression of a subproject in git for example. The idea to keep it a plugin is that it can be moved inside the lifecycle, and allows overriding / disabling / configuration as needed.

[gnodet-bot]

Re-review after 2cc8c28 — both outstanding findings are now addressed:

1. Symlink exclusion — ✅ Addressed. Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS) is now used in both the directory stream filter (line 165) and the final orphan check (line 202). Symlinks to directories are no longer followed, consistent with the Cleaner's default followSymlinks=false behavior.
2. Per-child IOException resilience — ✅ Addressed. The try/catch is now inside the for (Path child : children) loop, so an unreadable sibling logs a warning and scanning continues for remaining children.

All findings from all review cycles are resolved.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after 2cc8c28 — the new commit correctly addresses both Copilot findings:

1. Symlink handling — ✅ Addressed. Files.newDirectoryStream filter now uses Files.isDirectory(p, LinkOption.NOFOLLOW_LINKS) instead of the default Files::isDirectory. The isOrphanDirectory() final check also uses NOFOLLOW_LINKS. Symlinks are no longer followed, consistent with the Cleaner's default followSymlinks=false behavior.

2. IOException resilience — ✅ Addressed. isOrphanDirectory() failures are now caught per-child inside the loop (catch (IOException e) { logger.warn(...) }), so an unreadable sibling no longer aborts the scan of remaining children. The outer catch still handles failure to open the root stream.

All findings from all review cycles are resolved. Code is solid.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[rmannibucau]

@gnodet not sure what it does changes, if we do leak - and we do - and want to fix it, it is in core it belongs IMHO, clean can't be made aware of all of that nor if it changed. Typically the algo there is "any folder with target as single child", this is very fragile and will delete folder it shouldn't for ex - in particular cause it totally ignores the reactors cause the module is not more inside but also means it can visit folder with target dir which were always outside the reactor. At least core could ensure to put a target/.reactor with a timestamp or alike inside (or nothing but think some meta can help). Another issue is that a module which is "src+pom free" will be deleted (extensions enable to do that).

So really it is bound to core to me