Fix activity recovery and enforce 100% line coverage

.github/workflows/build.yml

@@ -74,3 +74,11 @@ jobs:
           name: unit-test-reports-${{ github.run_id }}-${{ github.run_attempt }}
           path: target/surefire-reports/
           if-no-files-found: error
+
+      - name: Upload coverage report
+        if: ${{ !cancelled() && hashFiles('target/site/jacoco/jacoco.xml') != '' }}
+        uses: actions/upload-artifact@v7
+        with:
+          name: coverage-report-${{ github.run_id }}-${{ github.run_attempt }}
+          path: target/site/jacoco/
+          if-no-files-found: error

README.md

@@ -35,10 +35,11 @@ mvn -B --no-transfer-progress clean verify
 ```
 
 JUnit 5 tests cover configuration loading, commands, the GUI, plugin hooks,
-listeners and activity progress, using proxy and Objenesis fixtures for server
-and plugin APIs. CI runs the same command on every push and pull request to `main` and
-uploads the Surefire reports; no coverage gate is enforced. The suite does not
-start a live Paper server.
+listeners and activity progress, using MockBukkit, real file fixtures, and scoped
+server/provider boundaries. Verification requires 100% production line coverage
+with no excluded classes, plus the JaCoCo execution data and XML report. CI runs
+the same command on every push and pull request to `main` and uploads the Surefire
+and coverage reports. The suite does not start a live Paper server.
 
 The description update tool has its own Python tests, which CI does not run:
 

pom.xml

@@ -30,7 +30,37 @@
         <vfbuilders.version>2.0.0</vfbuilders.version>
     </properties>
 
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>org.junit</groupId>
+                <artifactId>junit-bom</artifactId>
+                <version>5.11.4</version>
+                <type>pom</type>
+                <scope>import</scope>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+
     <dependencies>
+        <dependency>
+            <groupId>org.junit.platform</groupId>
+            <artifactId>junit-platform-launcher</artifactId>
+            <version>1.11.4</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockbukkit.mockbukkit</groupId>
+            <artifactId>mockbukkit-v1.21</artifactId>
+            <version>4.95.0</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>5.20.0</version>
+            <scope>test</scope>
+        </dependency>
         <!-- Paper API -->
         <dependency>
             <groupId>io.papermc.paper</groupId>
@@ -321,6 +351,63 @@
                 <artifactId>maven-surefire-plugin</artifactId>
                 <version>3.5.2</version>
             </plugin>
+            <plugin>
+                <groupId>org.jacoco</groupId>
+                <artifactId>jacoco-maven-plugin</artifactId>
+                <version>0.8.15</version>
+                <executions>
+                    <execution>
+                        <id>coverage-agent</id>
+                        <goals><goal>prepare-agent</goal></goals>
+                    </execution>
+                    <execution>
+                        <id>coverage-report</id>
+                        <phase>verify</phase>
+                        <goals><goal>report</goal></goals>
+                    </execution>
+                    <execution>
+                        <id>coverage-check</id>
+                        <phase>verify</phase>
+                        <goals><goal>check</goal></goals>
+                        <configuration>
+                            <rules>
+                                <rule>
+                                    <element>BUNDLE</element>
+                                    <limits>
+                                        <limit>
+                                            <counter>LINE</counter>
+                                            <value>MISSEDCOUNT</value>
+                                            <maximum>0</maximum>
+                                        </limit>
+                                    </limits>
+                                </rule>
+                            </rules>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-enforcer-plugin</artifactId>
+                <version>3.6.2</version>
+                <executions>
+                    <execution>
+                        <id>require-coverage-evidence</id>
+                        <phase>verify</phase>
+                        <goals><goal>enforce</goal></goals>
+                        <configuration>
+                            <rules>
+                                <requireFilesExist>
+                                    <files>
+                                        <file>${project.build.directory}/jacoco.exec</file>
+                                        <file>${project.reporting.outputDirectory}/jacoco/jacoco.xml</file>
+                                    </files>
+                                </requireFilesExist>
+                            </rules>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
     </build>
 </project>

src/main/java/net/tfminecraft/activitytf/ActivityPlugin.java

@@ -68,7 +68,11 @@ public void onDisable() {
 
         if (placeholderHook != null) {
             try {
-                placeholderHook.unregister();
+                // PlaceholderAPI removes by identifier; only remove the instance we registered.
+                if (placeholderHook.getPlaceholderAPI().getLocalExpansionManager()
+                    .getExpansion(placeholderHook.getIdentifier()) == placeholderHook) {
+                    placeholderHook.unregister();
+                }
             } catch (Throwable t) {
                 getLogger().warning("Failed to unregister the PlaceholderAPI expansion: " + t.getMessage());
             }
@@ -170,9 +174,13 @@ private void registerHooks() {
         }
 
         if (Bukkit.getPluginManager().isPluginEnabled("PlaceholderAPI")) {
-            placeholderHook = new PlaceholderHook(ActivityManager.getInstance());
-            placeholderHook.register();
-            getLogger().info("Hooked into PlaceholderAPI.");
+            PlaceholderHook hook = new PlaceholderHook(ActivityManager.getInstance());
+            if (hook.register()) {
+                placeholderHook = hook;
+                getLogger().info("Hooked into PlaceholderAPI.");
+            } else {
+                getLogger().warning("Failed to register the PlaceholderAPI expansion.");
+            }
         }
     }
 }

src/main/java/net/tfminecraft/activitytf/commands/ActivityCommand.java

@@ -79,34 +79,20 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
         }
 
         switch (sub) {
-            case "reload":
+            case "reload" -> {
                 manager.reload();
                 sender.sendMessage(messages().get("admin.reloaded"));
                 rewardStatus(sender);
                 audit(sender, "action=reload result=done");
-                return true;
-            case "rewards":
-                handleRewards(sender, args);
-                return true;
-            case "check":
-                handleCheck(sender, args);
-                return true;
-            case "reset":
-                handleReset(sender, args);
-                return true;
-            case "givereroll":
-                handleGiveReroll(sender, args);
-                return true;
-            case "add":
-                handleAdd(sender, args);
-                return true;
-            case "addpoints":
-                handleAddPoints(sender, args);
-                return true;
-            default:
-                usage(sender);
-                return true;
-        }
+            }
+            case "rewards" -> handleRewards(sender, args);
+            case "check" -> handleCheck(sender, args);
+            case "reset" -> handleReset(sender, args);
+            case "givereroll" -> handleGiveReroll(sender, args);
+            case "add" -> handleAdd(sender, args);
+            case "addpoints" -> handleAddPoints(sender, args);
+        }
+        return true;
     }
 
     private void handleRewards(CommandSender sender, String[] args) {

src/main/java/net/tfminecraft/activitytf/config/ActivityConfiguration.java

@@ -2,6 +2,7 @@
 
 import org.bukkit.Bukkit;
 import org.bukkit.Material;
+import org.bukkit.Sound;
 import org.bukkit.configuration.ConfigurationSection;
 import org.bukkit.configuration.file.FileConfiguration;
 import org.bukkit.configuration.file.YamlConfiguration;
@@ -1293,7 +1294,19 @@
         if (name == null || name.isBlank()) {
             return null;
         }
-        return name.trim().toLowerCase(Locale.ROOT).replace('_', '.');
+        String value = name.trim();
+        if (org.bukkit.NamespacedKey.fromString(value.toLowerCase(Locale.ROOT)) == null) {
+            log().warning("Invalid sound '" + value + "' - disabling it.");
+            return null;
+        }
+        if (value.indexOf('.') >= 0 || value.indexOf(':') >= 0) {
+            return value.toLowerCase(Locale.ROOT);
+        }
+        try {
+            return Sound.valueOf(value.toUpperCase(Locale.ROOT)).getKey().getKey();
+        } catch (IllegalArgumentException ignored) {
+            return value.toLowerCase(Locale.ROOT);
+        }
     }
 
     public Messages messages() {
@@ -1414,7 +1427,7 @@
     }
 
     static int nonVoteDailyMax(int dailyMax, int voteShare) {
-        return dailyMax * (100 - voteShare) / 100;
+        return (int) ((long) dailyMax * (100 - voteShare) / 100);
     }
 
     int parseVoteShare(ConfigurationSection config) {

src/main/java/net/tfminecraft/activitytf/gui/ActivityGui.java

@@ -380,12 +380,9 @@ private void reroll(Player player, InventoryClickEvent event) {
         ActivityConfiguration config = manager.getConfiguration();
         Messages messages = config.messages();
 
-        if (config.rerollsPerDay() <= 0) {
-            player.sendMessage(messages.get("reroll-disabled"));
-            return;
-        }
-
-        if (!player.hasPermission("activity.reroll")) {
+        // Disabled rerolls still report DISABLED before the permission check.
+        // The manager returns it before reading or changing player data.
+        if (config.rerollsPerDay() > 0 && !player.hasPermission("activity.reroll")) {
             player.sendMessage(messages.get("reroll-locked"));
             return;
         }

src/main/java/net/tfminecraft/activitytf/hooks/PlaceholderHook.java

@@ -60,7 +60,7 @@ public String onRequest(OfflinePlayer player, String params) {
             case "max":
                 return String.valueOf(config.barMax());
             case "percent":
-                return String.valueOf(points * 100 / config.barMax());
+                return String.valueOf(points * 100L / config.barMax());
             case "claimable":
                 return String.valueOf(currentWeek ? data.claimable(config.milestones()) : 0);
             case "daily_points":

src/main/java/net/tfminecraft/activitytf/hooks/TLibsItems.java

@@ -51,6 +51,12 @@ static boolean isUnresolved(Material type) {
             || type == Material.VOID_AIR || type == Material.DIRT;
     }
 
+    /** Retry unresolved paths after administrators reload item definitions. */
+    public static void reset() {
+        Default.INSTANCE.unresolvedPaths.clear();
+        Default.INSTANCE.reportedPaths.clear();
+    }
+
     public static ItemStack item(String path) {
         return Default.INSTANCE.resolve(path);
     }

src/main/java/net/tfminecraft/activitytf/listeners/BattleListener.java

@@ -20,10 +20,6 @@ public BattleListener(ActivityManager manager) {
     @EventHandler(priority = EventPriority.MONITOR)
     public void onBattleEnded(BattleEndedEvent event) {
         Set<UUID> participants = event.getParticipantIds();
-        if (participants == null) {
-            return;
-        }
-
         for (UUID participant : participants) {
             if (participant != null) {
                 manager.recordAction(participant, "battle_joined", 1);

src/main/java/net/tfminecraft/activitytf/listeners/ProfessionXpListener.java

@@ -22,10 +22,6 @@ public ProfessionXpListener(ActivityManager manager) {
 
     @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
     public void onExperienceGain(PlayerExperienceGainEvent event) {
-        if (!event.hasProfession()) {
-            return;
-        }
-
         Profession profession = event.getProfession();
         if (profession == null) {
             return;

src/main/java/net/tfminecraft/activitytf/managers/ActivityManager.java

@@ -803,9 +803,6 @@ void runClickCommandsNow(Player player, List<String> commands, String id, Predic
     }
 
     static List<String> runnableClickCommands(ActivityDef def, String name, int max, Logger logger) {
-        if (def == null) {
-            return List.of();
-        }
         List<String> runnable = new ArrayList<>();
         for (String command : def.clickCommands()) {
             if (!canRunCommand(command, name)) {
@@ -884,6 +881,7 @@ public void reload() {
         warnedEmptyPools.clear();
         reportedItemPaths.clear();
         ItemsAdderItems.reset();
+        TLibsItems.reset();
         reportedBrokenCommands.clear();
     }
 

src/main/java/net/tfminecraft/activitytf/store/PlayerStore.java

@@ -71,6 +71,9 @@ private boolean read() {
 
         ConfigurationSection root = yaml.getConfigurationSection("players");
         if (root == null) {
+            if (yaml.contains("players")) {
+                return quarantine("has a 'players' value that is not a section");
+            }
             Set<String> stray = yaml.getKeys(false);
             stray.remove("players");
             if (!stray.isEmpty()) {

src/test/java/net/tfminecraft/activitytf/ActivityAPITest.java

@@ -0,0 +1,28 @@
+package net.tfminecraft.activitytf;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+import java.util.UUID;
+import net.tfminecraft.activitytf.managers.ActivityManager;
+import org.bukkit.Bukkit;
+import org.junit.jupiter.api.Test;
+
+class ActivityAPITest {
+    @Test void mainThreadRecordsReachTheActiveManagerAndShutdownCallsAreIgnored() {
+        ActivityManager manager=mock(ActivityManager.class); UUID player=UUID.randomUUID();
+        try(var bukkit=mockStatic(Bukkit.class);var singleton=mockStatic(ActivityManager.class)) {
+            bukkit.when(Bukkit::isPrimaryThread).thenReturn(true);
+            singleton.when(ActivityManager::getInstance).thenReturn(manager);
+            ActivityAPI.record(player,"craft",7); verify(manager).recordAction(player,"craft",7);
+            singleton.when(ActivityManager::getInstance).thenReturn(null);
+            assertDoesNotThrow(()->ActivityAPI.record(player,"craft",7)); verifyNoMoreInteractions(manager);
+        }
+    }
+    @Test void asynchronousCallersAreRejectedBeforeLookingUpOrChangingPlayerState() {
+        try(var bukkit=mockStatic(Bukkit.class);var singleton=mockStatic(ActivityManager.class)) {
+            bukkit.when(Bukkit::isPrimaryThread).thenReturn(false);
+            assertThrows(IllegalStateException.class,()->ActivityAPI.record(UUID.randomUUID(),"craft",1));
+            singleton.verifyNoInteractions();
+        }
+    }
+}