Console modes: --console=plain/rich/verbose/machine

[gnodet]

Summary

Part 3 of the logging feature chain. Depends on #12695 (build report).

Adds the --console CLI flag with four output modes:

• plain — compact one-line-per-module output, ideal for CI (auto-selected in CI environments)
• rich — JLine status bar with live reactor progress (auto-selected on interactive TTYs)
• verbose — full mojo-level output, current Maven 4.0 default behavior
• machine — JSON lines: one typed JSON object per lifecycle event, designed for piping to external tools

Auto-detection (--console=auto, the default): CI → plain, interactive TTY → rich, otherwise → verbose.

Files changed (16 files, ~4000 insertions)

Area	Files
API	Options.java (+console())
Event loggers	PlainExecutionEventLogger, RichBuildEventListener, RichExecutionEventLogger, MachineBuildEventListener, MachineExecutionEventLogger
CLI wiring	CommonsCliOptions, LayeredOptions, LookupInvoker (preliminary interactive detection), MavenInvoker (console mode switch + transfer listener)
Existing logger	ExecutionEventLogger (version info on failure)
Tests	5 test classes with full coverage

PR chain

#	PR	Feature
1	#12694	Logging foundation
2	#12695	Build report
3	This PR	Console modes
4	#12698	Warning mode + diagnostics
5	#12699	mvnlog viewer
6	#12702	Structured problems pipeline
7	#12714	TRACE level migration

Test plan

• mvn test -pl impl/maven-cli — all 692 tests pass
• mvn test -pl impl/maven-core — all tests pass
• CI validation

🤖 Generated with Claude Code

[gnodet]

Well-structured PR adding four console modes with comprehensive test coverage (1317 lines of tests). Two issues found, one concurrency bug.

Also noted:

• The Options.console() API addition follows the established pattern (Optional<String>, same as color()). The @Experimental annotation means downstream breakage is acceptable.
• JSON escaping in MachineBuildEventListener correctly handles all RFC 8259 required escapes, with test coverage for edge cases.
• Test coverage is strong: 5 new test classes covering all four console modes.

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]

Race condition: completedProjects++ is not atomic. The field is volatile int (line 103) but ++ is a non-atomic read-increment-write compound operation. In parallel builds (-T N), concurrent calls to projectFinished() from worker threads can lose increments, causing the progress counter to under-report.

The same class already uses AtomicInteger for warningCount and errorCount (lines 133-136).

Suggested change

	completedProjects++;
	completedProjects.incrementAndGet();

(with completedProjects changed to AtomicInteger at line 103)

[gnodet]

Unrecognized --console values (including typos like --console=plian) silently fall through to auto-detection. This is inconsistent with the existing --color option in LookupInvoker.java (lines 274-276) which throws IllegalArgumentException for invalid values. Adding validation for consistency would help users catch configuration mistakes.

[gnodet]

Well-structured PR adding four console modes with comprehensive test coverage. Two concurrency issues found in RichBuildEventListener that affect parallel builds (-T flag):

Confirmed findings (verified independently):

1. [High — Concurrency bug] RichBuildEventListener.java:308 — completedProjects++ on a volatile int is not atomic. In parallel builds, multiple threads call projectFinished() concurrently, causing lost increments. The same class already uses AtomicInteger for warningCount (line 133) and errorCount (line 136), and the companion MachineExecutionEventLogger protects its equivalent counter with synchronized. completedProjects should use AtomicInteger as well.

2. [Medium — Logic issue in parallel builds] RichBuildEventListener.java:635 — projectOrder.indexOf(pid) < completedProjects assumes modules complete in reactor order, which does not hold in parallel builds. Example: reactor order [A(0), B(1), C(2)] with B and C independent. If C completes before B starts, completedProjects=2, and B (index 1 < 2) would incorrectly show a green checkmark before it runs. Consider tracking completed project IDs in a Set<String> instead of relying on positional count.

3. [Low — Style] MachineExecutionEventLogger.java:96 — session.getRequest().getGoals().stream().collect(Collectors.joining(" ")) can be simplified to String.join(" ", session.getRequest().getGoals()), consistent with line 104 of the same file which already uses String.join for profiles.

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]

[High — Concurrency bug] completedProjects++ on a volatile int is not atomic. In parallel builds (-T flag), multiple threads call projectFinished() concurrently, causing lost increments via the classic read-modify-write race.

This class already uses AtomicInteger for warningCount and errorCount. Suggest:

Suggested change

	completedProjects++;
	completedProjects.incrementAndGet();

(with the field declaration changed to private final AtomicInteger completedProjects = new AtomicInteger();)

[gnodet]

[Medium — Logic issue in parallel builds] This assumes modules complete in reactor order, which is not guaranteed with -T. A module not yet started can have a lower reactor index than the count of completed modules, causing an incorrect green checkmark.

Consider tracking completed project IDs in a Set<String> and checking completedProjects.contains(pid) instead.

[gnodet]

[Low — Style] This can be simplified to String.join(" ", session.getRequest().getGoals()), consistent with line 104 which already uses String.join for profiles.

Suggested change

	String goals = session.getRequest().getGoals().stream().collect(Collectors.joining(" "));
	String goals = String.join(" ", session.getRequest().getGoals());