Logging foundation: structured LogEvent, JUL handler, Log API enhancements

[gnodet]

Summary

Master-only logging infrastructure additions, layered on top of the forward-port (#12929) of the shared Log API enhancements from 4.0.x (#12690).

What's included (master-only)

Structured LogEvent (maven-api-core, maven-core)

• LogEvent / LogLevel API for structured log event representation
• Source metadata fields: sourceClassName(), sourceMethodName(), threadId() — populated for both Log API events (via DefaultLog.withMetadata()) and JUL events (via MavenJulHandler); null/-1 for direct SLF4J logging

Custom JUL Handler (maven-logging)

• MavenJulHandler replaces SLF4JBridgeHandler — all JUL events always route through SLF4J so that MavenSimpleLogger produces a consistent formattedMessage (with timestamp, logger name, and ANSI styling) regardless of origin
• JUL metadata (sourceClassName, sourceMethodName, threadId) is stashed in a ThreadLocal before the SLF4J call and read by ProjectBuildLogAppender during the same synchronous call chain — no metadata is lost
• Null loggerName guard per JUL spec (falls back to root logger)
• Three sources, one pipeline: Log API → SLF4J, SLF4J direct, JUL → MavenJulHandler → SLF4J — all converge on the same structured LogEvent

Structured LogSink (maven-logging, maven-core)

• MavenSimpleLogger.LogSink — structured callback with (level, loggerName, cleanMessage, formattedMessage, throwable) replacing the old Consumer<String> sink
• Throwable rendering unified: write() reuses the existing writeThrowable() method instead of duplicating rendering logic
• ProjectBuildLogAppender produces LogEvent objects (with source metadata when available) instead of raw strings
• BuildEventListener.projectLogMessage() now takes LogEvent instead of String

Log API metadata (maven-core)

• DefaultLog.withMetadata() — captures caller class/method/thread via StackWalker, gated behind ProjectBuildLogAppender.hasReportCapture() for zero overhead in normal builds (~1-5μs per-call cost only when build report capture is active)

Shared with 4.0.x (via forward-port #12929)

The following changes are in the forward-port commit and are not duplicated in this PR's diff:

• Log.trace() — default no-op methods preventing AbstractMethodError
• Log.child(name) — hierarchical sub-loggers
• Mojo MDC propagation (maven.mojo.id) — fork-aware save/restore
• Logger name alignment (FQCN instead of goal name)
• DefaultLog warn bug fix + isXxxEnabled() guards
• DefaultLogTest — 6 tests (warn regression, metadata lifecycle, trace delegation, trace no-op, child logger, default trace backward compat)

PR chain

#	PR	Feature
0	#12929	Forward-port of 4.0.x Log API enhancements
1	This PR	Logging foundation (master-only additions)
2	#12695	Build report
3	#12697	Console modes
4	#12698	Warning mode + diagnostics
5	#12699	mvnlog viewer
6	#12702	Structured problems pipeline
7	#12714	TRACE level migration

Related

• Log API enhancements and mojo MDC #12690 — merged into maven-4.0.x (milestone 4.1.0)
• Forward-port: Log API enhancements and mojo MDC #12929 — forward-port of Log API enhancements and mojo MDC #12690 to master (this PR is rebased on top)

Test plan

• DefaultLogTest — 6 tests: warn/supplier regression, metadata lifecycle, trace delegation, trace no-op, child logger, default trace backward compat
• MavenJulHandlerTest — 10 tests: parameterized JUL→SLF4J level mapping, FINEST→TRACE, CONFIG→INFO, metadata null check
• mvn test -pl impl/maven-core,impl/maven-logging — all tests pass
• Full CI validation
• IT suite with JUL-using plugins (verify metadata preservation)

🤖 Generated with Claude Code

[gnodet]

Well-designed foundational logging infrastructure. The structured LogEvent API, JUL handler, and Log API enhancements provide a solid base for the build report and console modes PRs. A few issues noted below.

Also noted:

• Good catch fixing warn(Supplier<String>, Throwable) calling logger.info() instead of logger.warn().
• The logger name change from getFullGoalName() to getImplementation() (FQCN) enables proper hierarchical SLF4J level configuration but is a behavioral change — worth mentioning in release notes for users who configured logging by short-form names.
• No unit tests were added for the new functionality (MavenJulHandler, DefaultLogEvent, StackWalker metadata capture, LogSink contract). Given this is foundational for the entire logging pipeline, targeted tests would increase confidence.

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

Claude Code on behalf of gnodet

[gnodet]

The @return Javadoc says "always non-negative" but the default implementation returns -1, and DefaultLogEvent convenience constructors also pass -1. The threadId() method in this same interface correctly documents its sentinel ("or -1 if unavailable").

Suggested change

	}
	* @return the sequence number, or {@code -1} if unavailable

[gnodet]

Fixed — javadoc now reads @return the sequence number, or {@code -1} if unavailable.

[gnodet]

Fixed — javadoc now reads @return the sequence number, or {@code -1} if unavailable.

[gnodet]

formatForConsole() produces [LEVEL] message (no timestamp, no logger name), while MavenSimpleLogger produces a full formatted line with timestamp and logger name per configuration. This means LogEvent.formattedMessage() has an inconsistent format depending on whether the event originated from JUL or SLF4J. The formattedMessage() Javadoc promises "the level prefix, timestamp, and any ANSI styling" — JUL-sourced events would be missing the timestamp and logger name.

[gnodet]

Fixed — removed the formatForConsole() method entirely. All JUL events now always route through SLF4J so that MavenSimpleLogger produces a consistent formattedMessage (with timestamp, logger name, and ANSI styling) regardless of origin. The JUL metadata is stashed in a ThreadLocal before the SLF4J call so that ProjectBuildLogAppender can read it when constructing the LogEvent.

[ascheman]

Really solid foundation — the three-path convergence (Log API / SLF4J / JUL) onto one structured LogEvent is clean, and preserving the LogRecord metadata the stock SLF4JBridgeHandler drops is a genuine improvement. Nice catch on the warn(Supplier, Throwable) → logger.info() bug.

A few things worth a look before this becomes the base of the 7-PR chain — one API-compat question, one fork-context correctness question, one perf note, and some small nits. Nothing structural.

On tests (echoing the earlier note): the two I'd most want are a regression test asserting warn(Supplier, Throwable) actually logs at WARN, and a table test for the JUL→SLF4J level mapping (esp. FINEST→TRACE and CONFIG→INFO). Given the ThreadLocal/StackWalker plumbing, those would lock down the easy-to-regress bits.

[ascheman]

The six new trace methods (isTraceEnabled + 5 overloads) are abstract, while child(String) below ships with a default. Any existing third-party implementor of Log breaks — source and binary (AbstractMethodError) — on trace, but not on child. Log is @Experimental, so it's arguably in-bounds for 4.1.0, but the asymmetry looks unintentional. Could the trace methods get backward-compatible defaults — isTraceEnabled() returning false and the trace(…) overloads as no-ops — so existing implementations keep compiling and opt in by overriding? (DefaultLog overrides all of it, so the runtime path is unchanged.)

[gnodet]

Fixed — all 6 trace methods (isTraceEnabled + 5 overloads) now have default implementations: isTraceEnabled() returns false by default, and all trace(...) overloads are no-ops. This prevents AbstractMethodError for existing third-party Log implementors.

[ascheman]

withMetadata runs a StackWalker.walk(…) on every enabled Log-API call to recover the caller method name. The isXxxEnabled() guards mean disabled levels are free, but INFO-level logging in a loop now pays a stack-walk per call. Worth a quick benchmark; alternatively make the source-method capture lazy/opt-in, since most appenders won't render it.

Minor, same method: Thread.currentThread().getId() is deprecated since Java 19 (Thread.threadId()), and the JUL path already uses the modern getLongThreadID() — aligning them would be consistent.

[gnodet]

Fixed — StackWalker is now conditional: if (ProjectBuildLogAppender.hasReportCapture()) before walking. The ~1-5μs per-call cost is only paid when build report capture is actually active. During normal builds, withMetadata() just sets the ThreadLocal with the logger name and thread ID (no stack walking).

[ascheman]

Project id has a fork-aware restore via FORKING_PROJECT_ID (consulted in getProjectId()), but mojo id doesn't: mojoSucceeded/mojoFailed in LoggingExecutionListener call setMojoId(null), so when a forking mojo (e.g. a report goal that forks a lifecycle) resumes after its fork completes, maven.mojo.id has been cleared and is never restored — subsequent log lines from the forking mojo lose their MDC attribution. Should there be a symmetric FORKING_MOJO_ID (or a save/restore around the fork) mirroring the project-id handling?

Related: MOJO_ID is an InheritableThreadLocal; with reused pool threads a value not cleared on an exceptional path could inherit into a later, unrelated task.

[gnodet]

Fixed — added FORKING_MOJO_ID ThreadLocal mirroring the existing FORKING_PROJECT_ID pattern. LoggingExecutionListener.forkStarted() saves the current mojoId via setForkingMojoId(), and forkSucceeded()/forkFailed() clear it. Also fixed the cleanup ordering: setMojoId(null) now runs after delegate.mojoSucceeded/mojoFailed so the mojo context is available during the delegate callback, and setMojoId(null) restores the forking mojo's ID if one was saved.

[ascheman]

record.getLoggerName() can be null per the JUL spec; it's passed straight to LoggerFactory.getLogger(loggerName). Worth confirming that resolves to the root logger across SLF4J bindings rather than producing a literally "null"-named logger.

[gnodet]

Fixed — added a null guard: if (loggerName == null) { loggerName = ""; }. SLF4J's LoggerFactory.getLogger("") returns the root logger, which is safe.

[ascheman]

The new LogSink path hand-rolls throwable rendering (appendFormattedThrowable/appendStackTrace) while the non-sink path still goes through super.write(…) → writeThrowable. Two throwable renderings that can drift over time — could the sink path reuse the existing writeThrowable/printStackTrace (which still take Consumer<String>) to keep them single-sourced? Relatedly, MavenJulHandler.formatForConsole emits a bare "[LEVEL] message" that won't match this class's console layout, so JUL-origin lines look different from native lines on the sink path.

[gnodet]

Fixed — unified throwable rendering: the write() method now calls the existing writeThrowable() method instead of hand-rolling its own appendFormattedThrowable()/appendStackTrace(). Removed the duplicate methods entirely — single rendering path now.

[ascheman]

sequenceNumber() javadoc says "always non-negative," but the default returns -1 and DefaultLogEvent passes -1 when unknown. Fix the doc (or the sentinel) so they agree.

[gnodet]

Fixed (same as @gnodet's comment above) — javadoc now reads @return the sequence number, or {@code -1} if unavailable.

[gnodet]

Well-designed logging infrastructure foundation with clean three-path convergence (Log API, JUL, SLF4J). The bug fix for warn(Supplier, Throwable) calling logger.info() is confirmed correct.

Findings:

1. [medium] sequenceNumber() javadoc/contract mismatch — LogEvent.sequenceNumber() javadoc says @return the sequence number, always non-negative but the default implementation returns -1. The sibling method threadId() correctly documents or -1 if unavailable in its @return tag. The sequenceNumber() javadoc should follow the same pattern for consistency.

2. [medium] Inconsistent formattedMessage format between JUL and SLF4J — When a LogSink is installed, JUL events' formattedMessage is built by formatForConsole() which produces a minimal [LEVEL] message string, while SLF4J events produce a full formatted string with timestamps, thread names, and logger names via MavenBaseLogger.innerHandleNormalizedLoggingCall(). The practical impact is limited since the clean message() field is available for consumers who need consistent content, but in SimpleBuildEventListener.projectLogMessage() which uses formattedMessage() for console output, JUL events will look noticeably different from SLF4J events.

3. [low] Log4j2/Logback backend removal — The removal of Log4j2Configuration, LogbackConfiguration, and the logback-classic dependency means Maven no longer supports these as alternative SLF4J backends. This is intentional for the Maven 4.x logging redesign, but warrants mention in release notes for users who embedded Maven with a custom logging backend.

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

Claude Code on behalf of Guillaume Nodet

[gnodet]

Well-architected logging foundation PR. Clean design with proper ThreadLocal management, volatile concurrency handling, and good layering (API → impl → collector). A few items to address:

High severity:

1. API contract contradiction (LogEvent.java line 188): sequenceNumber() Javadoc says "@return the sequence number, always non-negative" but the default implementation returns -1. Compare with threadId() which correctly documents "or -1 if unavailable". This is a public API interface marked @Experimental/@since 4.1.0 — the Javadoc should match the actual contract.

2. No test coverage: 1000+ lines of foundational code across 22 files with zero test files. LogEvent/DefaultLogEvent, MavenJulHandler (249 lines), DefaultLog.withMetadata/trace/child, LogSink interface, ProjectBuildLogAppender structured event creation, and the mojo MDC lifecycle are all untested. The PR description mentions "580 tests pass" but these are all pre-existing tests.

Medium severity:

3. StackWalker overhead (DefaultLog.java line 649): withMetadata() calls StackWalker.walk() on every log call for enabled levels. While trace/debug are typically disabled and info/warn/error are low-volume, plugins logging many INFO/WARN messages will pay the 1-5μs per-call cost.

4. Logger name change (DefaultBuildPluginManager.java line 128): Logger name changed from getFullGoalName() (e.g., "compiler:compile") to getImplementation() (e.g., "org.apache.maven.plugins.compiler.CompilerMojo"). Intentional for proper hierarchical SLF4J configuration, but a user-visible behavior change that could break existing SLF4J level configurations.

5. Dead code for future PR (ProjectBuildLogAppender.java line 130): reportCapture volatile field and setter are infrastructure for PR #12695 (build report). Currently unused in this PR — consider adding a brief comment noting the intent.

Low severity:

6. setMojoId(null) is called before delegate.mojoSucceeded/mojoFailed callbacks, inconsistent with the forkSucceeded/forkFailed pattern where cleanup happens after the delegate.

7. The bug fix changing logger.info() to logger.warn() in warn(Supplier<String>, Throwable) is correct and important. 👍

The removal of Logback/Log4j2 support is a significant architectural decision — worth explicit mention in release notes since users plugging in alternative SLF4J backends will lose that ability.

---

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

Claude Code on behalf of Guillaume Nodet

[gnodet]

Review feedback addressed

All 8 review comments from @gnodet and @ascheman have been addressed in the latest force-push. Summary of changes:

Bug fixes

• sequenceNumber() javadoc — corrected from "always non-negative" to "or -1 if unavailable"
• warn(Supplier, Throwable) regression — was calling logger.info() instead of logger.warn() (added regression test)

Design improvements

• MavenJulHandler simplified — removed formatForConsole() entirely; all JUL events now always route through SLF4J so MavenSimpleLogger produces a consistent formattedMessage regardless of origin
• Log.trace() backward compatibility — all 6 trace methods changed from abstract to default implementations (no-ops) to prevent AbstractMethodError for existing third-party Log implementors
• StackWalker gated — withMetadata() only walks the stack when ProjectBuildLogAppender.hasReportCapture() is true; normal builds pay zero StackWalker cost
• Fork-aware mojoId — added FORKING_MOJO_ID ThreadLocal mirroring FORKING_PROJECT_ID; forkStarted()/forkSucceeded()/forkFailed() save and restore mojo context
• Throwable rendering unified — MavenSimpleLogger.write() now reuses the existing writeThrowable() method; removed the duplicate appendFormattedThrowable()/appendStackTrace() methods
• Null loggerName guard — MavenJulHandler.publish() now handles null logger names per JUL spec

Tests added

• DefaultLogTest (4 tests): warn/supplier regression, metadata lifecycle, trace delegation, trace no-op
• MavenJulHandlerTest (10 tests): parameterized JUL→SLF4J level mapping, FINEST→TRACE, CONFIG→INFO, metadata null check

All 6 downstream PRs (#12695, #12697, #12698, #12699, #12702, #12714) have been rebased onto the updated commit.

[gnodet-bot]

All 8 findings from previous reviews have been verified as addressed in the latest commits:

1. sequenceNumber() javadoc — fixed: now reads @return the sequence number, or {@code -1} if unavailable
2. formatForConsole() inconsistency — fixed: removed entirely; all JUL events now route through SLF4J for consistent formattedMessage output
3. Log.trace() backward compatibility — fixed: all 6 trace methods are default no-ops, preventing AbstractMethodError for existing implementors
4. StackWalker overhead — fixed: gated behind ProjectBuildLogAppender.hasReportCapture(), zero cost during normal builds
5. Fork-aware mojoId — fixed: FORKING_MOJO_ID ThreadLocal added, mirroring FORKING_PROJECT_ID save/restore pattern in forkStarted/forkSucceeded/forkFailed
6. Null loggerName guard — fixed: MavenJulHandler.publish() now falls back to "" for null logger names per JUL spec
7. Throwable rendering unified — fixed: MavenSimpleLogger.write() reuses writeThrowable(), removed duplicate appendFormattedThrowable/appendStackTrace
8. mojoSucceeded/mojoFailed cleanup ordering — fixed: setMojoId(null) now runs after delegate.mojoSucceeded/mojoFailed callbacks

The ThreadLocal lifecycle management is correct throughout (always in try/finally blocks). The ConcurrentHashMap.computeIfAbsent reentrancy mitigation in MavenJulHandler (using putIfAbsent + IllegalStateException catch) is well-designed. Test coverage hits the key regression points (warn/Supplier bug, JUL level mapping, metadata lifecycle).

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

[gnodet-bot]

Re-review of commit f8d3a2e ("Fix MavenITmng4387 flakiness: activate logging before creating terminal").

The root-cause analysis in the commit message is correct and the primary fix (swapping activateLogging() before createTerminal()) eliminates the race at its source. One correctness gap in the secondary hardening and a missing test.

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

[gnodet-bot]

[medium] JUL quiet level should be SEVERE, not WARNING

In quiet mode, SLF4J is configured at ERROR level. The stated goal of the JUL root level assignment is to suppress events at source so that the brief window where newly-created SLF4J loggers are still at their default INFO level doesn't leak output. Using Level.WARNING only blocks INFO/DEBUG/FINE JUL events — WARNING events (JUL integer value 900) still pass the root filter and reach MavenJulHandler.publish(). Inside publish(), they hit isLevelEnabled(), which during that brief race window may see the SLF4J logger at INFO (not yet propagated to ERROR), allowing the WARNING event through to the console — exactly the flakiness pattern this fix is trying to prevent.

Level.SEVERE (integer value 1000) is the JUL equivalent of SLF4J ERROR and matches the quiet-mode intent precisely: only SEVERE-and-above JUL events pass the root filter.

Suggested change

	julRootLevel = java.util.logging.Level.WARNING;
	julRootLevel = java.util.logging.Level.SEVERE;

[gnodet]

Fixed in 2fc5fc8: quiet-mode JUL root level changed from WARNING to SEVERE (the correct JUL equivalent of SLF4J ERROR). Updated the comment accordingly.

[gnodet-bot]

[low] Update comment to reference SEVERE instead of WARNING

If Level.SEVERE is adopted above, the comment on line 460 ("keep JUL root at WARNING") should be updated to say SEVERE to stay accurate.

[gnodet]

Fixed in 9cc0d82. The comment on line 460 now reads keep the JUL root at SEVERE so that WARNING/INFO/DEBUG — updated together with the Level.WARNING → Level.SEVERE change.

[gnodet-bot]

Re-review of commit 1eefbf0 ("Fix MavenITmng4387 flakiness: guard JUL root level + CHM reentrancy").

The IN_PUBLISH reentrancy guard, the MavenStyleResolver.resolve() catch block, and the pendingEarlyLogs deferred-drain are all correct. One finding carried over from the previous review that was not addressed.

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

[gnodet-bot]

[medium] Quiet-mode JUL root level should be SEVERE, not WARNING (carried over from previous review — still unaddressed)

In quiet mode Maven sets SLF4J to ERROR level (Slf4jConfiguration.Level.ERROR, line 296). The stated goal of this JUL root level assignment is to suppress events at source so that newly-created SLF4J loggers (briefly at INFO before propagation) don't leak output. But Level.WARNING (integer 900) blocks only INFO/DEBUG/FINE JUL events — WARNING-and-above events still pass the root filter and reach MavenJulHandler.publish(). During the race window where a new SLF4J logger hasn't had ERROR propagated yet, a JUL WARNING event will slip through isLevelEnabled() (which sees INFO) and appear on console — exactly the flakiness pattern this is trying to fix.

Level.SEVERE (integer 1000) is the correct JUL equivalent of SLF4J ERROR. Update the code and the comment above:

Suggested change

	julRootLevel = java.util.logging.Level.WARNING;
	julRootLevel = java.util.logging.Level.SEVERE;

[gnodet]

Fixed in 2fc5fc8.

[gnodet-bot]

Re-review of commit 3752425 ("Fix javadoc: remove broken @link to LookupInvoker#createTerminal in LookupContext").

Two findings. The MavenITmng6065 fix is non-functional.

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

[gnodet-bot]

[high] pendingEarlyLogs drain is dead code — activateLogging() has not run yet when this executes

doInvoke() calls createTerminal() at line 159, then activateLogging() at line 160. The drain block below executes at the end of createTerminal(), but context.pendingEarlyLogs is set by activateLogging() (line 518), which hasn't been called yet. So context.pendingEarlyLogs is always null here, the if branch is never taken, and the MavenITmng6065 fix (early messages reaching the log file) is completely non-functional.

The fix has two correct options:

Option A — move the drain to the start of activateLogging() after installing ProjectBuildLogAppender (but that's the wrong place since the appender is installed in createTerminal()).

Option B — move the drain call to occur after activateLogging() returns, i.e. call it from doInvoke() directly:

    protected int doInvoke(C context) throws Exception {
        ...
        configureLogging(context);
        createTerminal(context);   // installs ProjectBuildLogAppender / logSink
        activateLogging(context);  // sets context.pendingEarlyLogs
        // Now that both the logSink AND pendingEarlyLogs are set, drain:
        if (context.pendingEarlyLogs != null) {
            context.pendingEarlyLogs.forEach(e -> context.logger.log(e.level(), e.message(), e.error()));
            context.pendingEarlyLogs = null;
        }
        helpOrVersionAndMayExit(context);
        ...
    }

The drain block currently in createTerminal() should be removed.

[gnodet]

Fixed in 63ed04e. Moved the drain into activateLogging() (after the context.logger.drain() / logger swap), where createTerminal() has already installed ProjectBuildLogAppender. The dead drain block in createTerminal() has been removed, and the pendingEarlyLogs field in LookupContext is gone too.

[gnodet-bot]

[low] Missing @param content tag — inconsistent with all other overloads

The diff removes @param content the message supplier from trace(Supplier<String> content). Every other supplier-based method in this interface (debug, info, warn, error) retains its @param content and @param error tags. The removal makes the Javadoc incomplete and inconsistent.

Suggested change

	default void trace(Supplier<String> content) {}
	/**
	* Sends a lazily-computed message at the <b>trace</b> error level.
	* The supplier is only evaluated if trace is enabled.
	* <p>
	* The default implementation is a no-op for backward compatibility
	* with existing {@code Log} implementations.
	*
	* @param content the message supplier
	*/
	default void trace(Supplier<String> content) {}

[gnodet]

Fixed in 63ed04e. Added @param content the message supplier to the Javadoc for trace(Supplier<String>).

[gnodet-bot]

[low] Missing @param content and @param error tags — inconsistent with all other overloads

Same issue: the diff removes both @param tags from trace(Supplier<String>, Throwable). Every other supplier+throwable method in this interface documents both parameters.

Suggested change

	default void trace(Supplier<String> content, Throwable error) {}
	/**
	* Sends a lazily-computed message (and accompanying exception) at the <b>trace</b> error level.
	* The supplier is only evaluated if trace is enabled.
	* <p>
	* The default implementation is a no-op for backward compatibility
	* with existing {@code Log} implementations.
	*
	* @param content the message supplier
	* @param error the error that caused this log
	*/
	default void trace(Supplier<String> content, Throwable error) {}

[gnodet]

Fixed in 63ed04e. Added both @param content the message supplier and @param error the error that caused this log to the Javadoc for trace(Supplier<String>, Throwable).

[gnodet-bot]

Re-review of commits 63ed04e ("Fix MavenITmng6065: drain pendingEarlyLogs in activateLogging()") and caeeeb4 ("Fix IT failures: replace computeIfAbsent with get+putIfAbsent in MavenStyleResolver").

Both findings from the previous REQUEST_CHANGES review are confirmed addressed:

1. pendingEarlyLogs drain — correctly moved into activateLogging() (after createTerminal() has installed ProjectBuildLogAppender), captured before context.logger is swapped, then replayed through the new logger. The ordering is now correct: drain → swap → replay.

2. Missing @param tags — trace(Supplier<String> content) and trace(Supplier<String> content, Throwable error) both now carry their @param tags matching the other overloads.

Three low-severity observations (not blocking merge):

• publishIsReentrantSafe test comment — the Javadoc says "Install a custom SLF4J logger that fires a second JUL event" but the test never does that; it only externally sets IN_PUBLISH=true then calls publish(). The assertion is correct and the guard is verified, but the comment misleads future readers about what scenario is actually being tested.

• DefaultLog.child(name) blank-name guard — requireNonNull prevents null but not blank. The previous interface @param said "must not be null or blank"; the updated doc just says "must not be null". A blank string produces a trailing-dot logger name ("com.example.Mojo. "). Harmless in practice but worth a Preconditions.checkArgument(!name.isBlank(), ...) if the contract is meant to exclude blanks.

• LogEvent.formattedMessage() Javadoc — says "the fully formatted log line" but when a throwable is present, MavenSimpleLogger.write() appends the ANSI-styled stack trace, so formattedMessage is actually a multi-line block. The doc could say "including any throwable rendering" to be precise.

None of these block this PR.

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

[gnodet-bot]

Re-review of commits 0c6eb24f ("Fix MavenITmng4387 Windows: eagerly reconfigure() in setRootLoggerLevel()") and 65cff258 ("Fix MavenITmng4387 Windows: set JUL root level early in configureLogging()").

Both commits are correct. Moving the JUL root level assignment for quiet/normal modes from activateLogging() into configureLogging() (before createTerminal() starts the FastTerminal background thread) closes the racy window cleanly. The verbose path staying deferred to activateLogging() is correct — setting ALL before MavenJulHandler is installed would flood the default ConsoleHandler. The if/else reorder in activateLogging() (verbose-first) is also cleaner. No new issues.

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