Improve standalone ApiRunner to properly apply settings

[gnodet]

Summary

The standalone ApiRunner is the main entry point for using the Maven 4 API outside of a full Maven build (e.g. in tools like mvnup). Previously it had several gaps that made it unsuitable for production use — settings were hardcoded to a test directory, proxy/mirror/auth from settings.xml were ignored, and dependency resolution machinery was not configured.

This PR fixes all of these issues:

• Remove hardcoded user.home = "target" — the standalone API now reads settings from the real user home (~/.m2/settings.xml)
• Store effective settings on the session — session.getSettings() now returns the actual effective settings instead of an empty object
• Apply proxy configuration from settings via DefaultProxySelector
• Apply mirror configuration from settings via DefaultMirrorSelector
• Apply server authentication from settings via DefaultAuthenticationSelector (username, password, private key)
• Configure dependency resolution machinery via MavenSessionBuilderSupplier — DependencyTraverser, DependencyManager, DependencySelector, DependencyGraphTransformer, ArtifactTypeRegistry, ArtifactDescriptorPolicy
• Detect and set Maven version from classpath pom.properties (fixes @Nonnull contract violation)
• Honor offline mode from settings
• Clean up stale TODO comments and dead commented-out code

Also makes 5 getter methods in MavenSessionBuilderSupplier public (from protected) so they can be reused by ApiRunner across packages.

Test plan

• All 611 tests in maven-impl pass (0 failures)
• TestApiStandalone — verifies artifact resolution and dependency collection work
• RequestTraceTest — verifies request tracing works with standalone session
• DiTest — verifies DI wiring
• maven-core compiles successfully with the visibility changes

🤖 Generated with Claude Code

[gnodet]

Solid enhancement that makes the standalone ApiRunner production-ready by wiring proxy, mirror, auth, and dependency resolution from settings.xml.

One item merits attention:

getMavenVersion() can still return null when pom.properties is absent from the classpath or version parsing fails, violating the @Nonnull contract declared in Session.java. The PR improves on the previous code (which always returned null in standalone mode) but doesn't fully close the gap. A fallback sentinel version (e.g. "0.0.0") would satisfy the contract.

Strengths:

• The proxy/mirror/auth configuration faithfully follows patterns from DefaultRepositorySystemSessionFactory and DefaultMavenExecutionRequestPopulator
• Clean removal of dead code and stale TODO comments
• Widening 5 MavenSessionBuilderSupplier methods from protected to public is appropriate — these are in the impl module, not the public API

Note: The commit message uses a placeholder JIRA number ([MNG-8xxx]). This should be replaced with a real issue number before merging.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Delta re-review — 3 new commits since previous review.

The three new commits are well-implemented improvements:

• Property loading (e484175, 333654f) correctly mirrors the CLI's precedence (installation system → user system → user properties) without needing ${includes} directive support. The ~/.m2/ location for user properties is correct per Maven conventions. Code comments clearly explain the design decisions.
• Repository handling (7521131) properly preserves settings-derived repositories in mvnup and only appends fallback defaults when not already present. The set-based ID check is the right approach.

Previous findings status

Finding	Status	Detail
getMavenVersion() null vs @Nonnull	⬆️ Improved	detectMavenVersion() now tries both maven-core and maven-impl pom.properties, but still returns null when neither exists. Pre-existing on master (previously always null in standalone mode), so the PR is an improvement. A fallback sentinel (e.g. VersionParser.parseVersion("0.0.0")) would fully close the gap.
Placeholder JIRA [MNG-8xxx]	⏳ Pending	First commit message still reads [MNG-8xxx] — should be replaced with a real issue number before merge.

New observation (low severity)

loadMavenProperties silently swallows IOException and returns an empty map. The class has no logger, so there's nowhere to log, but this could mask real configuration failures on files that exist but are unreadable (permissions, concurrent deletion). A debug-level log or comment explaining the rationale would help.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Delta re-review — All previous findings addressed.

✅ getMavenVersion() null contract — detectMavenVersion() now returns @Nonnull Version, falling back to a "0.0.0" sentinel when no pom.properties is on the classpath. Fully closes the @Nonnull contract gap.

✅ Placeholder JIRA [MNG-8xxx] — Removed from the rebased commit message.

✅ Silent IOException in loadMavenProperties — Catch block now has a clear 3-line comment explaining the design rationale: properties files are optional, standalone API has no logger, and failure gives the same behavior as if the file didn't exist.

Clean fix commit, well-scoped to exactly the reported issues. No new concerns.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Delta review — one new commit: 6d26d3fdec — "Support encrypted settings passwords in standalone ApiRunner"

Well-designed addition with a clean SecurityMode API, proper classpath detection, and support for both legacy ({...}) and Maven 4 master-key-based encryption.

Two minor observations (non-blocking):

1. hasExistingDispatchers guard (ApiRunner.java:252): The check uses getAllBindings(Dispatcher.class) which only finds unqualified bindings. The test-scoped SecDispatcherProvider registers named dispatchers (via @Named(LegacyDispatcher.NAME) etc.), so the guard doesn't actually detect them. In practice this is harmless (DI framework handles duplicates and dispatchers are stateless), but the check doesn't achieve its documented intent of preventing duplicate bindings in the test scenario.

2. SecurityMode test coverage (ApiRunner.java:143): The existing TestApiStandalone implicitly exercises the default IF_AVAILABLE_WARN path, but the NONE, IF_AVAILABLE, and REQUIRED modes are untested. In particular, the REQUIRED mode's MavenException throw when plexus-sec-dispatcher is absent has no test verifying the behavior.

Positive notes:

• Good design decision to call configureSecurityDispatchers before injectorConsumer.accept(injector), allowing callers to override default bindings.
• SecDispatcherBindings correctly mirrors the test-scoped SecDispatcherProvider, covering all four master source types.
• Visibility changes in MavenSessionBuilderSupplier (protected→public) are a reasonable follow-on from earlier commits.

---

_{🤖 This review was generated by ForgeBot.}

[gnodet]

Delta review — one new commit: d220f4e91c — "Add tests for ApiRunner settings handling and SecurityMode"

✅ SecurityMode test coverage — All four enum values (NONE, IF_AVAILABLE, IF_AVAILABLE_WARN, REQUIRED) are now exercised in the happy path. Settings features (servers, mirrors, repositories, offline, encrypted passwords) are well-tested with proper isolation. The REQUIRED mode's exception path when plexus-sec-dispatcher is absent remains untested, but the test comment explicitly acknowledges this limitation — the library is a compile dependency, so ClassNotFoundException cannot be triggered without classloader tricks. Reasonable trade-off.

Good use of the validated master/server password values from the mng-8379 integration test fixtures.

No new concerns.

---

_{🤖 This review was generated by ForgeBot.}