Skip to content

Commit 539eaad

Browse files
author
Mark Pollack
committed
Fail the build on SpotBugs findings of rank 9 or less
SpotBugs 4.10.4 runs spotbugs:check in verify on main sources of every module, effort Max, threshold Medium, maxRank 9, on JDK 17 and 21. The 4.10 Maven plugin requires Maven 3.8.9 and the wrapper is 3.8.6, so the 4.9.8.5 plugin runs the current engine. find-sec-bugs was evaluated and left out: no finding at rank 9 or less, and its lower findings are the library's purpose. The two UR_UNINIT_READ findings in the session constructors are false positives (the field is assigned through a method-reference callback during subscribe) and are excluded by class, method, field and pattern, with the reason, in config/spotbugs/spotbugs-exclude.xml.
1 parent aa44e3f commit 539eaad

3 files changed

Lines changed: 103 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,18 @@ Found by enabling Error Prone's bug checks; each has a test.
226226
request plumbing. The class-cycle rule in acp-core's `ArchitectureTest` now covers every package
227227
except `json` (whose default-mapper lookup is a cycle by design), with the `AcpAgent` <->
228228
`DefaultAcpAsyncAgent` cycle gone.
229+
- A bug-pattern gate fails `verify`: SpotBugs 4.10.4 (spotbugs-maven-plugin 4.9.8.5; the 4.10
230+
plugin needs Maven 3.8.9 and the wrapper is 3.8.6) runs `spotbugs:check` on main sources in every
231+
module, on JDK 17 and JDK 21 alike, at effort Max, and fails on any finding of rank 9 or less
232+
(SpotBugs' "scariest" and "scary" bands). Rank, not confidence alone, is the gate: it weighs the
233+
pattern's severity with its confidence, so the long tail of rank 10 to 20 advice (exposed record
234+
arrays and collections, inner classes that could be static, serialization hygiene) does not fail
235+
the build. find-sec-bugs was evaluated and not added: it raised nothing at rank 9 or less, and its
236+
findings below that are the library's purpose (starting the configured agent process, writing
237+
JSON-RPC to a servlet response). The first run found three: two false positives, excluded one by
238+
one with their reasons in `config/spotbugs/spotbugs-exclude.xml`, and the Streamable HTTP SSE
239+
stream locking on its own monitor, which any code holding the stream could also take; it now
240+
locks a private object.
229241

230242
## [0.18.0] - 2026-09-25
231243

‎config/spotbugs/spotbugs-exclude.xml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
ACP Java SDK SpotBugs exclusions (spotbugs-maven-plugin, `verify`, main sources, rank 9 or less).
4+
5+
Each entry is one false positive, named by class, method and bug pattern, with the reason it is
6+
false. Never exclude a whole pattern or package: a real instance of the same pattern elsewhere
7+
must still fail the build. A finding that is a real bug or a risky pattern is fixed in the code.
8+
9+
Exposure patterns (EI_EXPOSE_REP, EI_EXPOSE_REP2: records and value objects handing out the
10+
components they hold) are rank 18, outside the gate, so they need no entry here.
11+
-->
12+
<FindBugsFilter
13+
xmlns="https://github.com/spotbugs/filter/3.0.0"
14+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
15+
xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/3.1.0/spotbugs/etc/findbugsfilter.xsd">
16+
17+
<!--
18+
UR_UNINIT_READ (rank 3): "constructor reads a field which has not yet been assigned".
19+
The constructor starts the transport with this::onStartFailure as its error callback; a
20+
transport that refuses synchronously calls it during subscribe(), and it assigns
21+
startFailure. The read after subscribe() is how the constructor learns that, and fails
22+
construction instead of returning a session that can never talk. SpotBugs does not follow
23+
the assignment through the method reference, so it sees only the default value.
24+
-->
25+
<Match>
26+
<Class name="com.agentclientprotocol.sdk.spec.AcpAgentSession"/>
27+
<Method name="&lt;init&gt;"/>
28+
<Field name="startFailure"/>
29+
<Bug pattern="UR_UNINIT_READ"/>
30+
</Match>
31+
32+
<!--
33+
UR_UNINIT_READ (rank 3): the same construction as AcpAgentSession, on the client side:
34+
connect()'s error callback this::onConnectFailure assigns connectFailure synchronously when
35+
the transport refuses to connect (stdio does, and every transport does on a second connect).
36+
-->
37+
<Match>
38+
<Class name="com.agentclientprotocol.sdk.spec.AcpClientSession"/>
39+
<Method name="&lt;init&gt;"/>
40+
<Field name="connectFailure"/>
41+
<Bug pattern="UR_UNINIT_READ"/>
42+
</Match>
43+
44+
</FindBugsFilter>

‎pom.xml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@
9595
<maven-pmd-plugin.version>3.28.0</maven-pmd-plugin.version>
9696
<pmd.version>7.28.0</pmd.version>
9797
<cpd.minimumTokens>100</cpd.minimumTokens>
98+
<!-- spotbugs-maven-plugin 4.10.x requires Maven 3.8.9+, and the wrapper is 3.8.6; the
99+
4.9.8.x plugin runs the current SpotBugs engine below. -->
100+
<spotbugs-maven-plugin.version>4.9.8.5</spotbugs-maven-plugin.version>
101+
<spotbugs.version>4.10.4</spotbugs.version>
98102

99103
<!-- Error Prone and NullAway (errorprone profile, JDK 21+ only) -->
100104
<errorprone.version>2.50.0</errorprone.version>
@@ -389,6 +393,49 @@
389393
</executions>
390394
</plugin>
391395

396+
<!-- SpotBugs gate: spotbugs:check fails verify on any finding of rank 9 or less
397+
(SpotBugs' "scariest" 1-4 and "scary" 5-9 bands), on main sources, in every
398+
module and on every JDK the build runs on (17 and 21; unlike the errorprone
399+
profile it does not need JDK 21). Rank, not confidence, is the gate: rank
400+
combines the pattern's severity with its confidence, so it keeps the bugs and
401+
drops the long tail (exposed arrays, missing static on inner classes,
402+
serialization advice) that a confidence threshold alone would keep. threshold
403+
Medium (the default) also drops low-confidence findings; effort Max runs the
404+
interprocedural detectors. find-sec-bugs is not used: on this code base it adds
405+
no finding at rank 9 or less, and its findings below that are the library's
406+
purpose (spawning the configured agent process, writing JSON-RPC responses).
407+
A false positive is excluded narrowly (class, method, pattern), with its reason,
408+
in config/spotbugs/spotbugs-exclude.xml. -->
409+
<plugin>
410+
<groupId>com.github.spotbugs</groupId>
411+
<artifactId>spotbugs-maven-plugin</artifactId>
412+
<version>${spotbugs-maven-plugin.version}</version>
413+
<dependencies>
414+
<dependency>
415+
<groupId>com.github.spotbugs</groupId>
416+
<artifactId>spotbugs</artifactId>
417+
<version>${spotbugs.version}</version>
418+
</dependency>
419+
</dependencies>
420+
<configuration>
421+
<effort>Max</effort>
422+
<threshold>Medium</threshold>
423+
<maxRank>9</maxRank>
424+
<includeTests>false</includeTests>
425+
<excludeFilterFile>${maven.multiModuleProjectDirectory}/config/spotbugs/spotbugs-exclude.xml</excludeFilterFile>
426+
<failOnError>true</failOnError>
427+
</configuration>
428+
<executions>
429+
<execution>
430+
<id>spotbugs-gate</id>
431+
<phase>verify</phase>
432+
<goals>
433+
<goal>check</goal>
434+
</goals>
435+
</execution>
436+
</executions>
437+
</plugin>
438+
392439
<!-- JaCoCo code coverage -->
393440
<plugin>
394441
<groupId>org.jacoco</groupId>

0 commit comments

Comments
 (0)