fix(logging): stop logback.xml from shadowing logback-spring.xml

AGENTS.md

@@ -132,23 +132,44 @@ artifact ships the SBOM without per-image wiring.
 ### Logging Architecture
 
 The STDIO transport uses stdout for JSON-RPC messages, so any stray stdout output
-corrupts the protocol. Logging is configured in two layers:
-
-- **`logback.xml`** — Loaded by logback BEFORE Spring Boot initializes. Contains only
-  a `NopStatusListener` to suppress logback's internal status messages (`|-INFO`,
-  `|-WARN`) that would otherwise be written directly to stdout. Required for native
-  image where logback falls through to `BasicConfigurator` without it.
-- **`logback-spring.xml`** — Loaded by Spring Boot, overrides `logback.xml`. Uses
-  `<springProfile>` blocks to scope appenders per transport mode:
-  - **HTTP**: CONSOLE appender (stdout) + OpenTelemetry appender (OTLP log export with
-    `captureExperimentalAttributes` and `captureKeyValuePairAttributes` enabled).
-  - **STDIO**: No appenders defined. Relies on `logging.pattern.console=` in
-    `application-stdio.properties` to produce empty output from Spring Boot's default
-    console appender. The OTEL appender is intentionally excluded to keep stdout clean.
-- **`application-stdio.properties`** — Sets `logging.pattern.console=` (empty pattern)
-  which suppresses all Spring-managed console logging after Spring Boot initializes.
-
-**Init order**: logback.xml → Spring Boot starts → logback-spring.xml → application-{profile}.properties
+corrupts the protocol.
+
+**`logback-spring.xml` is the only logging configuration this project ships, and the
+`-spring` suffix is load-bearing.** Spring Boot resolves the *standard* Logback
+locations (`logback-test.xml`, `logback.xml`, …) first in
+`AbstractLoggingSystem.initializeWithConventions()`; if it finds one and
+`logging.file.name` is unset, it reinitializes from that file and returns, never
+loading the `-spring` variant. So adding a `logback.xml` alongside does not override
+`logback-spring.xml` — it **disables** it, silently dropping every `<springProfile>`
+appender. This is Boot's documented rule: `<springProfile>` "cannot be used in the
+standard `logback.xml` file because it is loaded too early."
+
+That trap was live in this repo: both files existed, so HTTP mode ran with no
+appenders at all — no console logs, no OTLP log export, and startup failures exited
+1 showing only the Spring banner. `LoggingConfigurationTest` now fails the build if
+any standard-location Logback file reappears.
+
+Contents of `logback-spring.xml`:
+
+- A `NopStatusListener` suppressing logback's internal status messages (`|-INFO`,
+  `|-WARN`), which are written straight to stdout and bypass the appenders.
+- `<springProfile>` blocks scoping appenders per transport mode:
+  - **HTTP**: Boot's own `console-appender.xml` (so `logging.pattern.console` /
+    `logging.charset.console` / `logging.threshold.console` behave as in a stock Boot
+    app) + OpenTelemetry appender (OTLP log export with `captureExperimentalAttributes`
+    and `captureKeyValuePairAttributes` enabled).
+  - **STDIO**: No appenders defined, so nothing can reach stdout. The OTEL appender is
+    intentionally excluded too.
+- `application-stdio.properties` additionally sets `logging.pattern.console=` (empty
+  pattern) as a second line of defence.
+
+`SolrNativeHints` registers `logback-spring.xml` as a native-image resource.
+
+**Init order**: Spring Boot starts → logback-spring.xml → application-{profile}.properties
+
+**Debugging tip**: if an HTTP-mode startup fails with no output, logging config is the
+first suspect — run with `LOGGING_CONFIG=classpath:logback-spring.xml` to force-load it
+and reveal the real stack trace.
 
 ### Docker image strategy
 

src/main/java/org/apache/solr/mcp/server/config/SolrNativeHints.java

@@ -122,12 +122,15 @@ public void registerHints(RuntimeHints hints, @Nullable ClassLoader classLoader)
 					"org.springaicommunity.mcp.context.DefaultMetaProvider",
 					MemberCategory.INVOKE_DECLARED_CONSTRUCTORS);
 
-			// Include logback.xml in the native image so logback's early
-			// initialization (before Spring Boot) finds it and applies the
-			// NopStatusListener. Without this, logback falls through to
-			// BasicConfigurator and writes status messages to stdout,
-			// corrupting the MCP STDIO JSON-RPC framing.
-			hints.resources().registerPattern("logback.xml");
+			// Include logback-spring.xml in the native image so Spring Boot can
+			// apply the per-profile appenders (and the NopStatusListener that
+			// keeps stdout clean for MCP STDIO JSON-RPC framing).
+			//
+			// This must NOT be named logback.xml: Spring Boot resolves the
+			// standard Logback locations first and, on finding one, never loads
+			// the -spring variant — silently dropping every <springProfile>
+			// block. See LoggingConfigurationTest.
+			hints.resources().registerPattern("logback-spring.xml");
 		}
 	}
 }

src/main/resources/logback-spring.xml

@@ -15,40 +15,51 @@
   See the License for the specific language governing permissions and
   limitations under the License.
 -->
+<!--
+    The ONLY logging configuration this application ships.
+
+    It must keep the "-spring" name. Spring Boot resolves the standard Logback
+    locations (logback-test.xml, logback.xml, ...) first in
+    AbstractLoggingSystem.initializeWithConventions(); if it finds one, it
+    reinitializes from that file and returns without ever loading this one.
+    Adding a logback.xml alongside this file therefore does not "override" it —
+    it disables it, taking every <springProfile> appender below with it. Boot's
+    reference documentation is explicit that <springProfile> "cannot be used in
+    the standard logback.xml file because it is loaded too early".
+
+    LoggingConfigurationTest enforces that no standard-location file reappears.
+-->
 <configuration>
     <!--
-        Suppress logback internal status messages (|-INFO, |-WARN lines).
-        Without this, logback writes status output directly to stdout during
-        initialization, BEFORE the profile-specific root logger is configured.
-        In STDIO mode this corrupts the MCP JSON-RPC framing.
+        Suppress logback's internal status messages (|-INFO, |-WARN lines),
+        which are written straight to stdout and bypass the appenders below.
+        In STDIO mode that output would corrupt the MCP JSON-RPC framing.
     -->
     <statusListener class="ch.qos.logback.core.status.NopStatusListener"/>
 
-    <!-- Import Spring Boot's default logging configuration -->
+    <!-- Spring Boot's conversion rules and CONSOLE_LOG_* properties -->
     <include resource="org/springframework/boot/logging/logback/defaults.xml"/>
 
     <!--
-        HTTP mode - Full observability with console logging and OTEL export
-        Used when running as HTTP server with PROFILES=http
+        HTTP mode - console logging plus OTLP log export.
 
-        Appenders are defined inside the profile block so they do not exist
-        under the STDIO profile. This avoids logback "not referenced" warnings
-        and keeps STDIO stdout completely clean.
-    -->
-    <!--
-        "http & !stdio": if both profiles are activated at once (PROFILES=stdio,http)
-        the CONSOLE appender would write diagnostics to stdout, corrupting the MCP
-        JSON-RPC stream that STDIO transports over it. Keeping stdio dominant here
-        means the combination degrades safely instead of silently breaking.
+        Appenders are declared inside the profile block so they do not exist
+        under the STDIO profile at all, which keeps STDIO stdout clean without
+        relying on a filter or an empty pattern.
+
+        "http & !stdio": if both profiles are activated at once
+        (PROFILES=stdio,http) the CONSOLE appender would write diagnostics to
+        stdout and corrupt the JSON-RPC stream that STDIO transports over it.
+        Keeping stdio dominant means the combination degrades safely instead of
+        silently breaking.
     -->
     <springProfile name="http &amp; !stdio">
-        <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
-            <encoder>
-                <pattern>${CONSOLE_LOG_PATTERN:-%d{yyyy-MM-dd HH:mm:ss.SSS} %5p --- [%15.15t] %-40.40logger{39} : %m%n}
-                </pattern>
-                <charset>UTF-8</charset>
-            </encoder>
-        </appender>
+        <!--
+            Spring Boot's own CONSOLE appender, so the logging.pattern.console,
+            logging.charset.console and logging.threshold.console properties
+            behave exactly as they do in a stock Boot application.
+        -->
+        <include resource="org/springframework/boot/logging/logback/console-appender.xml"/>
 
         <!--
             OpenTelemetry appender for log export.
@@ -68,10 +79,8 @@
     </springProfile>
 
     <!--
-        STDIO mode (default) - stdout suppressed via logging.pattern.console=
-        in application-stdio.properties. No logback-level overrides needed;
-        Spring Boot's empty console pattern keeps stdout clean for MCP
-        JSON-RPC framing.
+        STDIO mode (default) - deliberately no appenders, so nothing can reach
+        stdout and break the MCP JSON-RPC framing. application-stdio.properties
+        additionally sets logging.pattern.console= as a second line of defence.
     -->
-
-</configuration>
+</configuration>

src/test/java/org/apache/solr/mcp/server/config/LoggingConfigurationTest.java

@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.mcp.server.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.URL;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Guards the logging configuration against the file-shadowing trap described in
+ * Spring Boot's reference documentation.
+ *
+ * <p>
+ * Spring Boot resolves a Logback configuration in
+ * {@code AbstractLoggingSystem.initializeWithConventions()}. That method looks
+ * at the <em>standard</em> Logback locations first — {@code logback-test.xml},
+ * {@code logback.xml} and their Groovy variants. If one is found and no
+ * {@code logging.file.name} is set, Boot reinitializes from that file and
+ * returns, so {@code logback-spring.xml} is never consulted at all.
+ *
+ * <p>
+ * Shipping both files is therefore not "one overrides the other" — it silently
+ * disables the {@code -spring} file. In this application that meant the
+ * {@code http} profile lost its {@code CONSOLE} and {@code OTEL} appenders,
+ * which live in {@code logback-spring.xml} inside a {@code <springProfile>}
+ * block: HTTP mode emitted no log output whatsoever, and a failed startup
+ * exited 1 with nothing but the Spring banner.
+ *
+ * <p>
+ * Boot's reference documentation states the rule directly: <em>"We recommend
+ * that you use the -spring variant for your logging configuration (for example,
+ * logback-spring.xml rather than logback.xml). If you use standard
+ * configuration locations, Spring cannot completely control log
+ * initialization."</em> The {@code <springProfile>} extension in particular
+ * <em>"cannot be used in the standard logback.xml file because it is loaded too
+ * early"</em>.
+ *
+ * @see <a href=
+ *      "https://docs.spring.io/spring-boot/reference/features/logging.html#features.logging.custom-log-configuration">Spring
+ *      Boot — Custom Log Configuration</a>
+ */
+class LoggingConfigurationTest {
+
+	private static final String[] STANDARD_LOGBACK_LOCATIONS = {"logback-test.xml", "logback-test.groovy",
+			"logback.groovy", "logback.xml"};
+
+	/**
+	 * The {@code -spring} variant must be the configuration Spring Boot actually
+	 * loads, which requires that no standard-location file shadows it.
+	 */
+	@Test
+	void springVariantIsNotShadowedByAStandardLogbackConfiguration() {
+		ClassLoader classLoader = getClass().getClassLoader();
+
+		for (String location : STANDARD_LOGBACK_LOCATIONS) {
+			URL shadowing = classLoader.getResource(location);
+			assertThat(shadowing)
+					.as("%s is on the classpath and shadows logback-spring.xml: Spring Boot resolves it first "
+							+ "in AbstractLoggingSystem.initializeWithConventions() and never loads the -spring "
+							+ "variant, so every <springProfile> appender is silently dropped", location)
+					.isNull();
+		}
+	}
+
+	/** The configuration Spring Boot is expected to load must exist. */
+	@Test
+	void springVariantIsPresentOnTheClasspath() {
+		assertThat(getClass().getClassLoader().getResource("logback-spring.xml"))
+				.as("logback-spring.xml carries the per-profile appenders and must ship on the classpath").isNotNull();
+	}
+}

src/test/java/org/apache/solr/mcp/server/config/SolrNativeHintsTest.java

@@ -71,9 +71,11 @@ void registersMcpResponseRecordHints() {
 	}
 
 	@Test
-	void registersLogbackXmlResourceHint() {
-		// Required so logback's pre-Spring initialization finds logback.xml and
-		// stays silent on stdout (MCP STDIO framing).
-		assertTrue(RuntimeHintsPredicates.resource().forResource("logback.xml").test(hints));
+	void registersLogbackSpringXmlResourceHint() {
+		// logback-spring.xml is the only logging configuration we ship: a
+		// standard-location logback.xml would shadow it entirely (see
+		// LoggingConfigurationTest). It must be reachable in the native image so
+		// the per-profile appenders survive AOT.
+		assertTrue(RuntimeHintsPredicates.resource().forResource("logback-spring.xml").test(hints));
 	}
 }