[goutamadwant]

Follow-up to #13885, related to #13883. Targets maven-3.10.x.

The original shadowing fix is already merged. A custom rootDirectory that refers to ${project.rootDirectory} or ${pom.rootDirectory} can still fail interpolation because the recursion detector treats the custom property and the built-in expression as the same key.

Track bare rootDirectory separately and retain prefix-aware cycle detection for the other expressions. Tests cover model, user and system aliases, early model consumers, genuine cycles and tracker reset in both interpolators.

Validation

• New regressions fail on unchanged current maven-3.10.x.
• mvn -B -ntp verify: 1,154 tests reported, 8 skipped, zero failures or errors.
• Four focused Core IT cases pass with the fix; the explicit-root alias case fails on the current upstream distribution. Companion integration tests.
• Full Core IT suite and hosted CI have not been run.

Following this checklist to help us incorporate your
contribution quickly and easily:

• Your pull request should address just one issue, without pulling in other changes.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Each commit in the pull request should have a meaningful subject line and body.
  Note that commits might be squashed by a maintainer on merge.
• Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
  This may not always be possible but is a best-practice.
• Run mvn verify to make sure basic checks pass.
  A more thorough check will be performed on your pull request automatically.
• You have run the [Core IT][core-its] successfully.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004
• I hereby declare that I have filed an Apache Individual Contributor License Agreement.

[gnodet-bot]

LGTM. Sound fix for a real false-positive recursion detection bug.

The approach of routing bare rootDirectory to a separate SimpleRecursionInterceptor while keeping prefix-aware tracking for project.rootDirectory/pom.rootDirectory is correct — the expressions are logically distinct and the previous PrefixAwareRecursionInterceptor was stripping prefixes and colliding them. The state-routing via "rootDirectory".equals(expression) is deterministic, so push/pop pairs remain balanced within each interceptor.

Test coverage is thorough: aliases from model/user/system property sources × both prefixes, genuine cycle detection, namespace isolation, and clear() reset. Both StringSearchModelInterpolatorTest and StringVisitorModelInterpolatorTest extend AbstractModelInterpolatorTest, so the new tests run against both interpolator implementations.

⚠️ Pre-fix validation: dynamic check skipped (no maven-3.10.x worktree). Static trace shows testRootDirectoryAliases and testRootDirectoryRecursionNamespaces would fail on the unpatched base branch — PrefixAwareRecursionInterceptor.hasRecursiveExpression() strips the project./pom. prefix from both rootDirectory and project.rootDirectory, causing them to collide and produce a false-positive cycle. testRootDirectoryCycleIsDetected tests a genuine cycle (rootDirectory → other → rootDirectory) and is a safety-net test that likely passes on the unpatched code too — which is acceptable since it's guarding against a different scenario.

Suggested milestone: 3.10.1.

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

[slawekjaranowski]

I will check it

[slawekjaranowski]

Thanks for the follow-up! I checked out the PR and ran *ModelInterpolatorTest against both the patched and the base maven-3.10.x code. testRootDirectoryAliases and testRootDirectoryRecursionNamespaces fail without the fix and pass with it. 👍 (testRootDirectoryCycleIsDetected also passes without the fix, so it's a safety net rather than a regression test, which is fine.)

Two remaining points:

1. baseUri has the same problem. Like rootDirectory, it is registered with PrefixedValueSourceWrapper(..., PROJECT_PREFIXES, false), so only the prefixed form resolves to the built-in value. With this PR applied, <baseUri>${project.baseUri}</baseUri> still fails on both interpolators:

   Resolving expression: '${project.baseUri}': Detected the following recursive expression cycle in 'project.baseUri': [baseUri]
   

2. Cycle messages lose information. With two separate interceptors, a cycle that crosses both is reported only in part. For rootDirectory -> other -> rootDirectory, the message was [rootDirectory, other] before and is now just [rootDirectory].

The suggestions below replace the two interceptors with a single stack and a key function. project.rootDirectory and pom.rootDirectory share one key, while a bare rootDirectory or baseUri gets its own. Everything else keeps the current prefix-stripping behaviour, including basedir and model fields, where the unprefixed form is still a real alias. They also add regression tests for both points.

Please also add import org.codehaus.plexus.interpolation.util.ValueSourceUtils;, which the new code needs. Imports that are no longer used are removed by Spotless.

I tested these changes locally: mvn verify passes and Spotless leaves the suggested code unchanged.

Minor: in assertRootDirectoryInterpolation, expectedCustomRoot is always root.toString(), so the parameter can be dropped.

[goutamadwant]

@slawekjaranowski fixed both points using a single stack, with distinct built-in keys to avoid custom-property collisions. PTAL when possible. thanks!

[github-actions]

@cstamas Please assign appropriate label to PR according to the type of change.