fix!: ship a Java 8 loadable SLF4J binding and checker-qual - #411
Merged
Merged
Conversation
This artifact targets Java 8, but three of its runtime jars were Java 11 bytecode (class-file major 55), measured on the resolved runtime classpath: logback-classic 1.6.3, logback-core 1.6.3 and checker-qual 4.2.2. logback is the hard failure. SLF4J's ServiceLoader loads LogbackServiceProvider at JVM startup, so a Java 8 consumer got UnsupportedClassVersionError before any library code ran. The obvious fix -- drop to logback's Java 8 line -- is worse than the disease: 1.3.16 (2025-10-29) is its last release, CVE-2026-1225, CVE-2026-9828 and CVE-2026-10532 were fixed only in 1.5.x, and CVE-2026-19880 only in 1.6.3, which is Java 11 bytecode and therefore unreachable from here. That is a permanent unpatchable state, not a pin. slf4j-simple instead: six classes, same release train as slf4j-api, and no configuration parser, socket server or deserialization -- the subsystems essentially every logback CVE lives in. Consumers who want logback exclude it and declare their own binding, which is what the SLF4J split is for. checker-qual is the quieter one. Its annotations are @retention(RUNTIME), not CLASS, so anything reflecting over an annotated element loads them and a Java 8 JVM throws. <optional>true</optional> keeps it out of consumers' transitive graph but NOT out of the fat jar, because jar-with-dependencies filters on scope only -- so the pin to 3.55.1 (last major-52 release; the break is at 4.0.0) is what actually protects the shipped artifact. The build-time Checker Framework processor is decoupled onto its own property and stays at 4.2.2; collapsing them breaks the build, since 3.55.1 has no matching processor jar. Surefire now excludes slf4j-simple from the test classpath. Runtime scope is on the test classpath too, and LogCaptor requires logback specifically -- with both providers present, 7 tests failed with 'SLF4J Logger implementation should be of the type [ch.qos.logback.classic.Logger]'. Found by running them, not by reasoning about them. Verified: runtime classpath scanned for class-file majors above 52 -- 3 jars before, 0 after; the four LogCaptor test classes 74/74 green; spotbugs:check 0 bugs; spotless:check clean. BREAKING CHANGE: logback.xml is no longer read. Configure logging with a classpath simplelogger.properties or -Dorg.slf4j.simpleLogger.* system properties, or exclude org.slf4j:slf4j-simple and declare your own binding. With no configuration, output is quieter than before (logback defaulted to DEBUG, slf4j-simple defaults to INFO). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
bernardladenthin
had a problem deploying
to
maven-central
September 1, 2026 19:33 — with 
GitHub Actions
Failure
GitHub Actions
Failure
bernardladenthin
had a problem deploying
to
startgate
September 1, 2026 19:33 — with
GitHub Actions
Error
GitHub Actions
Error
bernardladenthin
had a problem deploying
to
maven-central
September 1, 2026 19:33 — with
GitHub Actions
Failure
GitHub Actions
Failure
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+62
to
+69
| <!-- Build-time Checker Framework processor. Runs on the CI JDK, never ships, so it | ||
| tracks the newest release. Deliberately NOT the same property as the annotations | ||
| below: those are shipped and must stay Java 8 bytecode. --> | ||
| <checker.version>4.2.2</checker.version> | ||
| <!-- Shipped checker-qual annotations. Last release whose classes are class-file | ||
| major 52; 4.0.0 moved the line to Java 11. See the dependency for why the pin, | ||
| not just the optional flag, is what protects the artifact. --> | ||
| <checker.qual.version>3.55.1</checker.qual.version> |
There was a problem hiding this comment.
Excellent separation of concerns here: checker.version (4.2.2, build-time only) stays current while checker.qual.version (3.55.1, shipped) is pinned to Java 8 bytecode. The comments clearly explain the why. ✓\n\nOne note: The comment at line 134 in dependencyManagement now says "we declare ${logback.version} directly" but logback-classic is no longer a direct dependency — it's now only a transitive from logcaptor (test scope). Consider updating that comment to clarify the transitive relationship and that the shipped artifact uses slf4j-simple instead."
…umers Replacing logback left the runnable jar with slf4j-simple's built-in defaults: WARN-and-above, to stderr, no timestamps. For a server whose output people pipe and correlate, that is a downgrade from what logback emitted. Adding src/main/resources/simplelogger.properties would fix the fat jar and break the library: it would be published inside llama.jar, and slf4j-simple reads whichever simplelogger.properties the classloader finds first -- so a consumer with their own file would get a coin flip decided by classpath order. A library does not get to make that choice for its consumers; an application does, and the fat jar is the application. So the file lives in src/main/assembly-resources/ and is added by a project descriptor (src/assembly/fat-jar.xml) instead of the predefined jar-with-dependencies ref. The descriptor's dependencySet is a verbatim copy of the predefined one -- outputDirectory /, useProjectArtifact true, unpack true, scope runtime -- and its comment says so, because the only reason the file exists is the fileSet below it. Defaults chosen to match what the logback binding used to produce: INFO, timestamps, thread and short logger name; stdout rather than slf4j-simple's stderr, so the server's own output shares a stream with everything else. All overridable with -Dorg.slf4j.simpleLogger.<key>. Verified by inspecting both built artifacts, not by reasoning about the descriptor: the fat jar contains simplelogger.properties and the slf4j-simple classes; the library jar contains neither; both are class-file major 52 throughout; no logback classes in either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
bernardladenthin
had a problem deploying
to
maven-central
September 1, 2026 19:40 — with
GitHub Actions
Failure
GitHub Actions
Failure
bernardladenthin
had a problem deploying
to
maven-central
September 1, 2026 19:40 — with
GitHub Actions
Failure
GitHub Actions
Failure
Review SummaryThis PR correctly fixes a hard failure where Java 8 consumers encounter UnsupportedClassVersionError at startup. Strengths:
Code quality: No bugs found. Assembly descriptor and properties files are structurally correct. Status: Local tests 74/74 passing. CI validation pending per PR checklist. Recommend merge once CI validates green. |
Comment on lines
2206
to
+2211
| <artifactId>maven-assembly-plugin</artifactId> | ||
| <configuration> | ||
| <descriptorRefs> | ||
| <descriptorRef>jar-with-dependencies</descriptorRef> | ||
| </descriptorRefs> | ||
| <!-- Our own descriptor, not the predefined jar-with-dependencies ref: it is | ||
| a verbatim copy of that one plus simplelogger.properties, which must | ||
| reach the runnable jar without being published in the library jar. --> | ||
| <descriptors> |
There was a problem hiding this comment.
Key design decision: Using a custom descriptor instead of the predefined jar-with-dependencies allows simplelogger.properties to be added to the fat jar only, not the library jar. This prevents the library from imposing logging configuration on its consumers — a critical distinction for library code. The custom descriptor is otherwise identical to the predefined one.
Comment on lines
+62
to
70
| <!-- Build-time Checker Framework processor. Runs on the CI JDK, never ships, so it | ||
| tracks the newest release. Deliberately NOT the same property as the annotations | ||
| below: those are shipped and must stay Java 8 bytecode. --> | ||
| <checker.version>4.2.2</checker.version> | ||
| <!-- Shipped checker-qual annotations. Last release whose classes are class-file | ||
| major 52; 4.0.0 moved the line to Java 11. See the dependency for why the pin, | ||
| not just the optional flag, is what protects the artifact. --> | ||
| <checker.qual.version>3.55.1</checker.qual.version> | ||
| <jackson.version>2.22.2</jackson.version> |
There was a problem hiding this comment.
Important split: checker.version (4.2.2) is for the build-time annotation processor, which runs on the CI JDK and never ships. checker.qual.version (3.55.1) is for the shipped annotations — it must stay on the last major-52 (Java 8) release because @Retention(RUNTIME) annotations are loaded by any reflection (Jackson does this during introspection). The <optional>true</optional> flag alone is insufficient because jar-with-dependencies filters on scope only, not the optional flag. Both the version pin AND the optional flag together protect the artifact.
Comment on lines
691
to
+712
| <configuration> | ||
| <!-- -XX:+EnableDynamicAgentLoading: silences the JDK 21 byte-buddy self-attach agent warning that intermittently corrupts Surefire's fork channel ("Corrupted channel ..." / bogus "timeout in the fork"). See workspace policy ci-test-diagnostics.md section 2.1: https://github.com/bernardladenthin/workspace/blob/main/policies/ci-test-diagnostics.md - root cause: https://github.com/raphw/byte-buddy/issues/1639 --> | ||
| <argLine>@{argLine} -Xmx2g -XX:ErrorFile=hs_err_pid%p.log -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=. -XX:+EnableDynamicAgentLoading</argLine> | ||
| <!-- | ||
| Keep the shipped SLF4J binding OFF the test classpath. | ||
|
|
||
| slf4j-simple is a runtime-scope dependency, and runtime scope is part of | ||
| the test classpath too. LogCaptor (io.github.hakky54, test scope) brings | ||
| logback, so both providers would be present, SLF4J would pick one | ||
| arbitrarily, and LogCaptor fails outright when it does not get logback: | ||
| "SLF4J Logger implementation should be of the type | ||
| [ch.qos.logback.classic.Logger] but found [org.slf4j.simple.SimpleLogger]" | ||
| (7 tests across LoggingSmokeTest, TimingsLoggerTest, ChatResponseParserTest | ||
| and CompletionResponseParserTest). | ||
|
|
||
| Excluding it here leaves logback as the sole provider during tests and does | ||
| not touch the artifact: the shipped jar and the fat jar still carry | ||
| slf4j-simple. The tests assert what our code logs, which is binding-agnostic; | ||
| which binding renders it is not what they are about. | ||
| --> | ||
| <classpathDependencyExcludes> | ||
| <classpathDependencyExclude>org.slf4j:slf4j-simple</classpathDependencyExclude> |
There was a problem hiding this comment.
Test-specific fix: Runtime-scoped dependencies are included on the test classpath. With both slf4j-simple and logback present, SLF4J picks a provider arbitrarily, causing LogCaptor (which requires logback specifically) to fail with "SLF4J Logger implementation should be of the type [ch.qos.logback.classic.Logger]". Excluding slf4j-simple here leaves logback as the sole provider in tests, while the shipped artifacts (both library and fat jar) still correctly carry slf4j-simple. Tests validate behavior, not binding choice.
This was referenced Sep 3, 2026
vaiju1981
pushed a commit
to vaiju1981/java-llama.cpp
that referenced
this pull request
Sep 15, 2026
main does not compile. bernardladenthin#411 pinned the shipped checker-qual to 3.55.1, the last release whose classes are Java 8 bytecode, so that a Java 8 consumer could not hit UnsupportedClassVersionError on annotations Jackson loads reflectively. The reasoning about the artifact was right; the mechanism was wrong. The Nullness Checker resolves its own qualifiers through javac's symbol table — the COMPILE CLASSPATH — not through the annotation-processor path. Under the 4.2.2 processor a 3.x checker-qual is missing org.checkerframework.framework.qual.DoesNotUnrefineReceiver, and every build dies with "Configuration problem! Could not load type: ...". Processor and qualifiers must share a major version. Reproduced both ways locally: main fails, main with -Dchecker.qual.version=4.2.2 succeeds. The publish run for bernardladenthin#411 was cancelled 26s in, so nothing caught it. provided scope satisfies both constraints at once instead of trading one for the other. 4.2.2 is on the compile classpath where the checker needs it, and provided is excluded from consumers' transitive graph AND from the fat jar (jar-with-dependencies takes scope runtime) — so no checker-qual class of any version reaches a consumer's JVM, which is strictly better than shipping an old one. <optional>true</optional> could not have done this on its own: that descriptor filters on scope only, which is exactly the trap bernardladenthin#411 documented. Safe because no source in this module imports org.checkerframework; the annotations exist for the processor, not for our code. Verified on the built artifacts: 0 checkerframework entries in the fat jar, mvn verify green (1474 tests), and the reactor's langchain4j and kotlin modules still build. The now-redundant checker.qual.version property is removed and the rationale rewritten in place, so the next reader sees why the obvious fix is not the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
vaiju1981
pushed a commit
to vaiju1981/java-llama.cpp
that referenced
this pull request
Sep 15, 2026
…FlashAttn Two separate failures were stacked on the `Java Tests Ubuntu` job, and the first was hiding the second. 1. PIT aborted with "5 tests did not pass without mutation ... requires a green suite" — on a suite Surefire had just reported green (1729 tests, 0 failures). Cause: two SLF4J providers on the test classpath. slf4j-simple arrives at runtime scope because it is the binding shipped in the fat jar, logback-classic at test scope because LogCaptor requires logback specifically; SLF4J's ServiceLoader picks one arbitrarily and every LogCaptor assertion fails when it picks slf4j-simple. bernardladenthin#411 added the binding and the Surefire `classpathDependencyExcludes` for exactly this, but PIT builds its OWN classpath and never reads Surefire's configuration, so the mutation run still saw both. pitest-maven has the same parameter, modelled on Surefire's; it is now set to match. Nothing about the artifact changes. It went unseen because every publish run between bernardladenthin#411 and now was cancelled; run 897 is the first to complete since. 2. With that cleared, PIT actually ran and surfaced the survivor that run 887 had already reported before bernardladenthin#411 ever landed: "Mutation score of 99 is below threshold of 100". `net.ladenthin.llama.args.*` is on the PIT target list at threshold 100, and FlashAttn — added in bernardladenthin#408 — was the one enum in that package with no test, so `getArgValue()`'s `return argValue` had no killer. Adds FlashAttnTest in the shape every sibling enum already uses. The values are a wire contract, not labels: since llama.cpp b10273 `--flash-attn` takes a mandatory value, so a wrong or empty token makes the parser eat the next argv entry and the load fails naming a flag the caller never set. Verified locally, both directions: the failure reproduces on the parent commit with the exact same five test names, and with the fix PIT reports 319/319 mutations killed (100%) and `mvn clean verify` is green at 1486 tests. CLAUDE.md now names the PIT half of the rule explicitly, next to the existing "spotbugs/spotless bind to verify" trap, including why srcmorph solves the same clash by excluding at the dependency instead (there both providers arrive transitively). This does NOT address the macOS 15 failures in the same run; those are a separate, bump-introduced regression and are reported alongside this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The artifact says Java 8 and ships Java 11 bytecode
llamacompiles withrelease 8, but three jars on its runtime classpath are class-file major 55. Measured, not assumed — every.classin every runtime jar, excludingmodule-info.classandMETA-INF/versions/**(a Java 8 classpath never reads those):After this PR: 0 jars above major 52.
logback is the hard failure, and downgrading is worse than the disease
SLF4J's
ServiceLoaderloadsLogbackServiceProviderat JVM startup, so a Java 8 consumer getsUnsupportedClassVersionErrorbefore a single line of library code runs.The obvious fix — drop to logback's Java 8 line — does not survive scrutiny:
1.3.16 (2025-10-29) is the last release of that line; five releases have shipped in 1.5/1.6 since, several security-driven, with no backports. Pinning it would trade a startup crash for permanent unpatchability, and Dependabot/osv-scanner would flag it forever with no upgrade path.
slf4j-simpleinstead: six classes, same release train as theslf4j-apialready depended on (so they cannot drift apart), and — the part that matters — no configuration parser, socket server or deserialization. Those are the subsystems essentially every logback CVE lives in (Janino expression evaluation,HardenedObjectInputStream,SaxEventRecorder,SocketReceiver); a binding without them cannot have that class of defect.checker-qual: the premise I started from was wrong
I assumed its annotations were
@Retention(CLASS)— "never loaded, so the version is irrelevant and<optional>true</optional>is enough". Checked instead of believed: 364 of them areRUNTIME, none areCLASS. Anything reflecting over an annotated element loads them, and Jackson does exactly that during introspection.And
<optional>true</optional>does not remove it from the fat jar — thejar-with-dependenciesdescriptor filters on<scope>only, verified against the descriptor shipped insidemaven-assembly-plugin-3.8.0.jar. So the optional flag protects consumers' transitive graph, and the version pin is what protects the shipped artifact. Both are applied.checker.versionis split into two properties.${checker.version}also drove the build-time Checker Framework processor inannotationProcessorPath; downgrading that broke resolution outright (org.checkerframework:checker:jar:3.55.1does not exist). The processor runs on the CI JDK and never ships, so it stays at 4.2.2 under its own property; only the shipped annotations move to 3.55.1 (the last major-52 release; the break is exactly at 4.0.0).One breakage found by running the tests, not by reasoning about them
Runtime scope is on the test classpath too, so slf4j-simple joined the logback that LogCaptor brings. Two providers, SLF4J picks one arbitrarily, and LogCaptor fails outright:
7 tests across
LoggingSmokeTest,TimingsLoggerTest,ChatResponseParserTestandCompletionResponseParserTest. Fixed with a surefireclasspathDependencyExcludesonorg.slf4j:slf4j-simple, which leaves logback the sole provider in tests and does not touch the artifact. The tests assert what our code logs, which is binding-agnostic.Test plan
mvn -f llama/pom.xml -DskipTests compile spotbugs:check—BugInstance size is 0(the exact command theCode stylejob runs)mvn -f llama/pom.xml spotless:check— exit 0pom.xmlverified well-formed by a strict XML parser after every editRelated issues / PRs
The same two problems exist in
srcmorphandstreambuffer(both Java 8);BitcoinAddressFinderis Java 21 and unaffected. A sharedverify-bytecode-version.shgate across all four repos follows separately — this class of defect is invisible to compilation, to the tests, and to every gate we currently run.Checklist
CONTRIBUTING.mdandCODE_OF_CONDUCT.mdfix!+BREAKING CHANGEtrailer)🤖 Generated with Claude Code
https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
Generated by Claude Code