Effective POM Resolution Refactor

[adesjardin]

• Refactor Effective pom.xml resolution to not call Maven for speed purposes
• No longer has to download and resolve all plugins and dependencies, only the parent poms

[Copilot]

Pull request overview

Refactors “effective POM” handling away from invoking an external Maven process by introducing embedded parent-POM resolution (via Maven Resolver/Aether) and adding new SPI types to expose inherited properties/dependencyManagement/pluginManagement data.

Changes:

• Added ParentPomResolver, SettingsXmlParser, and ParentPomResolutionException to resolve/capture parent POM chains using Maven Resolver and settings.xml.
• Enhanced PomFile with lazy parent-chain resolution and new resolveProperty/resolveDependency/resolvePlugin APIs plus new resolved-* model types.
• Removed Maven Invoker usage from MuleApplication/mule-linter-core and updated docs + test resources/assertions for the new parent sample structure.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
plans/04-parent-pom-resolution.md	Design plan for embedded parent POM resolution and new SPI APIs.
mule-linter-spi/src/main/groovy/com/avioconsulting/mule/linter/resolver/SettingsXmlParser.groovy	Loads Maven settings.xml (user/global) for resolver configuration.
mule-linter-spi/src/main/groovy/com/avioconsulting/mule/linter/resolver/ParentPomResolver.groovy	New resolver that downloads/locates parent POMs and builds parent chains.
mule-linter-spi/src/main/groovy/com/avioconsulting/mule/linter/resolver/ParentPomResolutionException.groovy	Custom exception to report resolution attempts/diagnostics.
mule-linter-spi/src/main/groovy/com/avioconsulting/mule/linter/model/pom/ResolvedProperty.groovy	New resolved property type with inheritance/source metadata.
mule-linter-spi/src/main/groovy/com/avioconsulting/mule/linter/model/pom/ResolvedDependency.groovy	New resolved dependency type with inheritance/source metadata.
mule-linter-spi/src/main/groovy/com/avioconsulting/mule/linter/model/pom/ResolvedPlugin.groovy	New resolved plugin type with inheritance/source metadata.
mule-linter-spi/src/main/groovy/com/avioconsulting/mule/linter/model/pom/PomFile.groovy	Adds lazy parent resolution + resolve* APIs that traverse parents.
mule-linter-spi/build.gradle	Adds Maven Resolver + Maven settings dependencies to SPI module.
mule-linter-core/src/test/resources/ComprehensiveParentSample/child/pom.xml	Updates relativePath to match the sample folder layout.
mule-linter-core/src/test/groovy/.../PomManagementRuleTest.groovy	Updates relativePath assertion for the sample structure.
mule-linter-core/src/test/groovy/.../EffectivePomIntegrationTest.groovy	Updates relativePath assertion for the sample structure.
mule-linter-core/src/main/groovy/.../MuleApplication.groovy	Removes Maven Invoker effective-pom generation and uses raw pom.xml parsing.
mule-linter-core/build.gradle	Removes maven-invoker dependency.
AGENTS.md	Documents new parent resolution behavior and new resolve* APIs.
.github/workflows/build.yml	Adds workflow env var intended to affect JS action runtime selection.

---

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

[Copilot]

resolvePlugin() returns immediately when a plugin is declared in <build><plugins> even if it has no <version>. In that common case Maven expects the version to come from <pluginManagement> (in this POM or a parent), but this early return produces a ResolvedPlugin with version == null and never consults pluginManagement. Please, when localPlugin exists but has no version, fall through to the pluginManagement lookup and mark the source accordingly.

[Copilot]

When the version comes from a parent’s <dependencyManagement>, the resolved result should keep the child dependency declaration and only source the version from the parent. Here the code constructs dependency from the parent management entry (new PomDependency(parentManaged, current)), which loses child-only details (scope/exclusions/type) and misattributes the dependency to the parent. Consider returning the child PomDependency (when present) and only sourcing version/versionSource from management.

Suggested change

	result.dependency = new PomDependency(parentManaged, current)
	result.dependency = localDep ?: new PomDependency(parentManaged, current)

[Copilot]

The error message tries to report whether the attempted relativePath exists using new File(attemptedPaths[0]), but attemptedPaths contains the raw relative path string, so the existence check will be evaluated relative to the current working directory (and will often be wrong). Consider passing the resolved absolute file path (relativePath resolved against the child POM directory) into the exception for accurate diagnostics.

[Copilot]

Docs mention system property mule.linter.localRepo to override the local repository path, but the current implementation never reads this property (no references in code) and ParentPomResolver always defaults to ${user.home}/.m2/repository unless constructed with an explicit argument. Either implement the documented system property (and/or honor settings.localRepository) or update the docs to match the actual behavior.

[Copilot]

This PR introduces substantial new behavior (remote/relative parent POM resolution, settings.xml handling, caching, circular detection), but there are no corresponding unit/integration tests added for ParentPomResolver (no ParentPomResolverTest found in the test suite). Please add focused tests for relativePath resolution, cached local repo usage, and failure/circular cases (mocking transport where needed) to prevent regressions and avoid network-dependent flakes.

[Copilot]

createSession() returns a fresh session but never configures it with a LocalRepositoryManager (e.g., system.newLocalRepositoryManager(session, localRepo)) or otherwise applies localRepositoryDir. Without that, resolveArtifact() commonly fails and/or won’t use the intended local cache. Please initialize a DefaultRepositorySystemSession, set the local repo manager, and return that configured session.

[Copilot]

ResolvedProperty.isFullyResolved() checks rawValue for ${...} markers when rawValue != value. If resolution substitutes into value, rawValue will still contain ${...}, so this will incorrectly report false even when value is fully resolved. It should check the resolved value (and/or resolutionChain) instead of rawValue.

Suggested change

	if (!rawValue || rawValue == value) {
	return true
	}
	return !rawValue.contains('${')
	if (!value || rawValue == value) {
	return true
	}
	return !value.contains('${')

[Copilot]

localRepositoryDir is always set to ~/.m2/repository, even when a custom local repo is configured (e.g., via a resolver argument, settings.xml, or a documented system property). This makes the exception diagnostics misleading. Please pass the actual local repo directory used by ParentPomResolver into the exception (or accept it as a constructor parameter).

[Copilot]

hasSettings() references settings.repositories, but Maven Settings does not expose repositories directly (they’re defined under profiles). If this method is called, it will throw a MissingPropertyException. Consider checking settings.profiles / settings.activeProfiles and/or mirrors/servers/proxies/localRepository instead.

Suggested change

	* Checks if settings.xml was loaded successfully.
	* @param settings The settings object
	* @return true if at least one settings file was parsed
	*/
	boolean hasSettings(Settings settings) {
	return settings != null &&
	(settings.servers?.size() > 0 ||
	settings.repositories?.size() > 0 ||
	settings.proxies?.size() > 0)
	* Checks if settings.xml contains any configured values.
	* @param settings The settings object
	* @return true if settings contains at least one configured element
	*/
	boolean hasSettings(Settings settings) {
	return settings != null &&
	(settings.localRepository ||
	settings.servers?.size() > 0 ||
	settings.proxies?.size() > 0 ||
	settings.mirrors?.size() > 0 ||
	settings.activeProfiles?.size() > 0 ||
	settings.profiles?.size() > 0)

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 12 comments.

---

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

[Copilot]

ParentPomResolver loads settings.xml but the computed localRepositoryDir ignores settings.localRepository (and SettingsXmlParser.getLocalRepositoryPath). As a result, users with a non-default local repo configured in Maven settings will be silently ignored; consider using the settings localRepository as the default (with system property/constructor still overriding).

Suggested change

	* @param localRepoPath Optional custom local repository path. Defaults to ~/.m2/repository
	* or value from 'mule.linter.localRepo' system property
	*/
	ParentPomResolver(String localRepoPath = null) {
	// Priority: 1) explicit parameter, 2) system property, 3) default ~/.m2/repository
	String effectivePath = localRepoPath ?:
	System.getProperty('mule.linter.localRepo') ?:
	"${System.getProperty('user.home')}/.m2/repository"
	this.localRepositoryDir = new File(effectivePath)
	this.settingsParser = new SettingsXmlParser()
	this.settings = settingsParser.loadSettings()
	* @param localRepoPath Optional custom local repository path. Defaults to ~/.m2/repository,
	* value from 'mule.linter.localRepo' system property, or Maven settings.xml
	* localRepository value
	*/
	ParentPomResolver(String localRepoPath = null) {
	this.settingsParser = new SettingsXmlParser()
	this.settings = settingsParser.loadSettings()
	// Priority: 1) explicit parameter, 2) system property, 3) settings.xml localRepository,
	// 4) default ~/.m2/repository
	String effectivePath = localRepoPath ?:
	System.getProperty('mule.linter.localRepo') ?:
	settingsParser.getLocalRepositoryPath(settings) ?:
	settings?.localRepository ?:
	"${System.getProperty('user.home')}/.m2/repository"
	this.localRepositoryDir = new File(effectivePath)

[Copilot]

buildRemoteRepositories() always returns Maven Central and does not apply any authentication/proxy/mirror info from settings.xml (despite the class-level docs/imports implying support). This will prevent resolving parent POMs from private repositories that require credentials; wire SettingsXmlParser.getServerAuthentication(...) into RemoteRepository.Builder#setAuthentication(...) (and consider proxies/mirrors as needed).

Suggested change

	private List<RemoteRepository> buildRemoteRepositories(List<String> attemptedRepos) {
	List<RemoteRepository> repos = []
	// Add Maven Central as default
	// Note: settings.xml profile repositories, mirrors, and proxies are not currently supported.
	// Only local repository path and server authentication are supported from settings.xml.
	repos.add(new RemoteRepository.Builder('central', 'default',
	'https://repo.maven.apache.org/maven2/').build())
	private org.eclipse.aether.repository.Authentication getRepositoryAuthentication(String repositoryId) {
	def settingsProperty = this.metaClass.hasProperty(this, 'settings')
	if (!settingsProperty) {
	return null
	}
	Settings effectiveSettings = settingsProperty.getProperty(this) as Settings
	if (!effectiveSettings?.servers) {
	return null
	}
	def server = effectiveSettings.servers.find { it?.id == repositoryId }
	if (!server) {
	return null
	}
	AuthenticationBuilder authenticationBuilder = new AuthenticationBuilder()
	if (server.username) {
	authenticationBuilder.addUsername(server.username)
	}
	if (server.password) {
	authenticationBuilder.addPassword(server.password)
	}
	return authenticationBuilder.build()
	}
	private List<RemoteRepository> buildRemoteRepositories(List<String> attemptedRepos) {
	List<RemoteRepository> repos = []
	// Add Maven Central as default.
	// Note: settings.xml profile repositories, mirrors, and proxies are not currently supported.
	// Server authentication from settings.xml is applied when a matching server id exists.
	RemoteRepository.Builder centralBuilder = new RemoteRepository.Builder('central', 'default',
	'https://repo.maven.apache.org/maven2/')
	def centralAuthentication = getRepositoryAuthentication('central')
	if (centralAuthentication != null) {
	centralBuilder.setAuthentication(centralAuthentication)
	}
	repos.add(centralBuilder.build())

[Copilot]

resolveParents swallows all exceptions and only prints to stderr, which can lead to silently incomplete inheritance and incorrect rule results (especially now that effective-POM generation was removed). Consider either propagating ParentPomResolutionException (fail-fast) or providing a configurable mode/flag that lets callers choose between fail-fast and “warn and continue”, and use a consistent logging mechanism rather than System.err.

[Copilot]

This plan lists a constraint “Must be testable with constructor injection (no singletons/statics)”, but the current implementation introduces ParentPomResolver.getInstance() singleton usage. Either update the plan to reflect the chosen approach, or remove the singleton in favor of injected instances as described here.

[Copilot]

These tests hard-code POSIX /tmp/... paths, which will fail on Windows runners and can also collide across concurrent test runs. Prefer using Files.createTempDirectory(...) / File.createTempDir() (and cleaning it up) to make the tests portable and isolated.

[Copilot]

close() claims the session is AutoCloseable, but RepositorySystemSession / DefaultRepositorySystemSession is not Closeable in practice, so this method is a no-op and tests calling it won’t actually release resources. Either remove close()/its comment, or track and close any actual closeable resources created by the resolver (e.g., transporter/http client) so callers can reliably clean up.

Suggested change

	* Closes the resolver and cleans up resources.
	*/
	void close() {
	// RepositorySystemSession is auto-closeable
	if (session instanceof Closeable) {
	((Closeable) session).close()
	}
	* No-op.
	*
	* This resolver does not currently own any closeable resources. The
	* RepositorySystemSession created by Maven Resolver is not Closeable in
	* normal use, so there is nothing to release here.
	*/
	void close() {
	// Intentionally empty.

[Copilot]

The class-level comment says parent resolution is “lazy - only performed when resolve methods are called”, but resolveProperty/resolveDependency/resolvePlugin never trigger resolveParents(...) and will just see parent == null unless callers remembered to call resolveParents beforehand. Either update the documentation to match the actual contract, or change the resolve* methods to ensure the parent chain is resolved automatically (e.g., by storing a resolver/factory on PomFile).

[Copilot]

AGENTS.md states that parent resolution “Uses ~/.m2/settings.xml for repository configuration and authentication”, but the current resolver implementation only adds Maven Central and does not apply settings-derived repositories/authentication. Update this documentation to match current behavior, or implement the documented settings.xml authentication support in ParentPomResolver.

Suggested change

	- Uses ~/.m2/settings.xml for repository configuration and authentication
	- Adds Maven Central for remote parent POM resolution; does not currently apply repository or authentication settings from ~/.m2/settings.xml

[Copilot]

This assertion is effectively a no-op (chain.size() >= 0 is always true), so the test will pass even if parent resolution is completely broken. Replace it with a meaningful expectation (e.g., chain.size() == 1 for the sample, or assert that the first parent has the expected coordinates/path) or remove the test if it cannot be deterministic in CI.

Suggested change

	// The parent should be parsed with MuleXmlParser (Issue #6 verification)
	chain.size() >= 0 // May be 0 if parent not resolvable in test environment
	// The sample fixture has a single parent and should resolve deterministically in CI.
	chain.size() == 1

[Copilot]

MuleApplication uses the shared ParentPomResolver.getInstance() but never closes it. If the resolver allocates resources (HTTP client connections, threads, caches), this can leak across long-running CLI/plugin executions and test suites. Consider owning the resolver lifecycle here (create per-application and close in a shutdown hook / explicit close() on MuleApplication) or make it clear that ParentPomResolver is intentionally process-global and resource-safe.

[Copilot]

Pull request overview

Copilot reviewed 30 out of 31 changed files in this pull request and generated 8 comments.

---

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

[Copilot]

localRepositoryDir is declared private, but it’s accessed from other classes/tests (e.g., PomFile.resolveParents() and resolver tests). This will break compilation. Please expose it via a public/protected getter (e.g., File getLocalRepositoryDir()) or adjust visibility so callers can read the effective local repo path for diagnostics/tests.

[Copilot]

resolveParents() only short-circuits when parent != null. For POMs with no <parent> (or when resolution yields an empty chain), repeated calls will keep re-running resolver logic (potentially re-hitting disk/network). Consider tracking a separate boolean like parentsResolved / parentResolutionAttempted so subsequent calls are no-ops even when there is no parent.

[Copilot]

resolver.localRepositoryDir is referenced here, but ParentPomResolver.localRepositoryDir is currently private, so this access will not compile. Please use a public getter on ParentPomResolver (or pass the effective local repo dir into resolveParents() / the exception constructor without reaching into private state).

Suggested change

	null, null, [], [], resolver.localRepositoryDir, e)
	null, null, [], [], resolver.getLocalRepositoryDir(), e)

[Copilot]

getGroupId() / getVersion() only read the direct <groupId> / <version> elements. In Maven, child POMs often omit these and inherit them from <parent>. Returning an empty string leads to incorrect sourceCoordinates (e.g., :artifactId:). Consider falling back to pomXml.parent.groupId / pomXml.parent.version when the direct elements are missing/blank (without requiring full parent resolution).

Suggested change

	/**
	* Returns the groupId from the POM
	*/
	String getGroupId() {
	return exists ? pomXml.getProperty('groupId') : ''
	}
	/**
	* Returns the version from the POM
	*/
	String getVersion() {
	return exists ? pomXml.getProperty('version') : ''
	private String getPomValueOrParentFallback(String propertyName) {
	if (!exists) {
	return ''
	}
	String value = pomXml.getProperty(propertyName) as String
	if (value?.trim()) {
	return value
	}
	GPathResult parentNode = pomXml.parent
	if (parentNode?.isEmpty()) {
	return ''
	}
	String parentValue = parentNode.getProperty(propertyName) as String
	return parentValue?.trim() ? parentValue : ''
	}
	/**
	* Returns the groupId from the POM
	*/
	String getGroupId() {
	return getPomValueOrParentFallback('groupId')
	}
	/**
	* Returns the version from the POM
	*/
	String getVersion() {
	return getPomValueOrParentFallback('version')

[Copilot]

Markdown formatting: **System Properties: is missing the closing **, so the rest of the section may render incorrectly. Please change it to **System Properties:**.

Suggested change

	**System Properties:
	**System Properties:**