feat: replace [pin] on initial kick and fix pinCheck boot warning (#33)

confuser · web-flow · commit 3239eed89634 · 2026-04-04T13:47:13.000+02:00
* feat: replace [pin] on initial kick and fix pinCheck boot warning

Listen to PlayerBannedEvent on all 6 platforms to call handlePin when a
kickMessage is present, enabling [pin] replacement on the first kick of
an online player (not just reconnect denial).

Change ExpiresSync from extends BmRunnable to implements Runnable to
eliminate the spurious "unknown last checked pinCheck" log on every boot.

Add e2e tests verifying the kick message contains a 6-digit pin when
banning/tempbanning an online player. Add waitForKick() helper to
TestBot with proper listener cleanup and race-safe settlement.

* fix: add [pin] placeholder to ban/tempban kick message templates

The e2e message configs only had [pin] in the disallowed (reconnect
denial) templates but not in the kick (online player) templates,
causing the Online Kick Pin tests to fail.

* fix: improve e2e test resilience for kick and denial tests

- Handle NBT compound chat components in kick reasons (Paper 1.20.5+)
- Add delay in waitForKick end handler to avoid socketClosed race
- Increase denial retry attempts and sleeps for async ban persistence
- Disconnect bot before unban in afterEach to avoid stale kick on reconnect

* fix: prevent stale SNAPSHOT dependencies in CI

- Add --refresh-dependencies to build step to force fresh SNAPSHOT resolution
- Use run_id in Loom cache key to prevent serving stale remapped dependency JARs

* fix: compile against BanManager from source and clean up bot on kick

- Checkout BanManager and publishToMavenLocal in both CI workflows so
  WebEnhancer always compiles against the latest API (Fabric Stonecutter
  artifacts are not reliably published to Maven Central snapshots)
- Clean up mineflayer bot (removeAllListeners + end) on kick/error during
  connect() and waitForKick() to prevent physics timers running after
  Jest teardown

* fix: force fresh BanManager dependency resolution and await bot cleanup

- Clear BanManager artifacts from Gradle and Loom caches before build
  so publishToMavenLocal takes priority over stale cached SNAPSHOTs
- Add --refresh-dependencies to Gradle build commands to bypass
  dependency resolution cache entirely
- Wait for mineflayer 'end' event before resolving waitForKick() and
  connect() cleanup, so the physics timer is cleared before Jest
  teardown begins (prevents prismarine-physics ReferenceError)
- Stop calling removeAllListeners() before bot.end() which stripped
  mineflayer's internal cleanup handlers

* chore: bump version to 7.11.0-SNAPSHOT to match BanManager

* fix: remove --refresh-dependencies from Fabric remapJar step to avoid OOM

Dependencies are already freshly resolved by the preceding build step.
The redundant --refresh-dependencies on the memory-intensive Loom
remapping step caused OutOfMemoryError.

* fix: improve denied-pin test cleanup and timing for Fabric

- Clear both permanent and temp bans in afterEach (bmunban + bmuntempban)
- Disconnect bot before unban to avoid stale state
- Add 2s sleep after cleanup for async processing to settle
- Increase sleep after bmtempban from 3s to 5s for Fabric enforcement lag

* fix: use short connect timeout in denied connection checks

On Fabric 1.21.4, tempbanned players sometimes enter a limbo state
(connected but neither spawned nor kicked) consuming the full 30s
connect timeout per attempt. Use a 10s timeout with 8 retries instead,
and increase the test timeout to 120s for headroom.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -26,6 +26,12 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - uses: actions/checkout@v4
+        with:
+          repository: BanManagement/BanManager
+          path: BanManager
+          ref: master
+
       - name: Set up JDK 17
         uses: actions/setup-java@v4
         with:
@@ -50,10 +56,19 @@ jobs:
           key: ${{ runner.os }}-loom-${{ hashFiles('**/libs.versions.*', '**/*.gradle*', '**/gradle-wrapper.properties') }}
           restore-keys: ${{ runner.os }}-loom-
 
+      - name: Publish BanManager to Maven Local
+        working-directory: BanManager
+        run: ./gradlew publishToMavenLocal --build-cache
+
+      - name: Clear stale BanManager dependency caches
+        run: |
+          find ~/.gradle/caches -path "*/me.confuser.banmanager*" -exec rm -rf {} + 2>/dev/null || true
+          find .gradle/loom-cache -path "*/BanManager*" -exec rm -rf {} + 2>/dev/null || true
+
       - name: Execute Gradle build
         env:
           STORAGE_TYPE: ${{ matrix.storageType }}
-        run: ./gradlew build --build-cache --info
+        run: ./gradlew build --build-cache --refresh-dependencies --info
 
       - name: Build all Fabric versions
         run: ./gradlew :fabric:1.20.1:remapJar :fabric:1.21.1:remapJar :fabric:1.21.4:remapJar :fabric:1.21.11:remapJar --build-cache
diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml
@@ -135,6 +135,15 @@ jobs:
           key: ${{ runner.os }}-loom-${{ hashFiles('**/libs.versions.*', '**/*.gradle*', '**/gradle-wrapper.properties') }}
           restore-keys: ${{ runner.os }}-loom-
 
+      - name: Publish BanManager to Maven Local
+        working-directory: BanManager
+        run: ./gradlew publishToMavenLocal --build-cache
+
+      - name: Clear stale BanManager dependency caches
+        run: |
+          find ~/.gradle/caches -path "*/me.confuser.banmanager*" -exec rm -rf {} + 2>/dev/null || true
+          find .gradle/loom-cache -path "*/BanManager*" -exec rm -rf {} + 2>/dev/null || true
+
       # Docker Buildx for better caching
       - name: Set up Docker Buildx
         uses: docker/setup-buildx-action@v3
@@ -171,7 +180,7 @@ jobs:
         run: ./gradlew ${{ matrix.build_task }} --build-cache
 
       - name: Run E2E tests
-        run: ./gradlew :BanManagerWebEnhancerE2E:${{ matrix.task }} --build-cache -PbanManagerPath=BanManager
+        run: ./gradlew :BanManagerWebEnhancerE2E:${{ matrix.task }} --build-cache --refresh-dependencies -PbanManagerPath=BanManager
         timeout-minutes: 15
         env:
           MC_VERSION: ${{ matrix.mc_version }}
diff --git a/bukkit/src/main/java/me/confuser/banmanager/webenhancer/bukkit/listeners/ReportListener.java b/bukkit/src/main/java/me/confuser/banmanager/webenhancer/bukkit/listeners/ReportListener.java
@@ -1,10 +1,12 @@
 package me.confuser.banmanager.webenhancer.bukkit.listeners;
 
 import me.confuser.banmanager.common.ormlite.stmt.DeleteBuilder;
+import me.confuser.banmanager.bukkit.api.events.PlayerBannedEvent;
 import me.confuser.banmanager.bukkit.api.events.PlayerReportDeletedEvent;
 import me.confuser.banmanager.bukkit.api.events.PlayerReportedEvent;
 import me.confuser.banmanager.bukkit.api.events.PlayerDeniedEvent;
 import me.confuser.banmanager.bukkit.api.events.PluginReloadedEvent;
+import me.confuser.banmanager.common.util.Message;
 import me.confuser.banmanager.common.data.PlayerReportData;
 import me.confuser.banmanager.webenhancer.bukkit.BukkitPlugin;
 import me.confuser.banmanager.webenhancer.common.data.LogData;
@@ -76,6 +78,17 @@ public void onDeny(PlayerDeniedEvent event) {
     listener.handlePin(event.getPlayer(), event.getMessage());
   }
 
+  @EventHandler(priority = EventPriority.MONITOR)
+  public void onBanned(PlayerBannedEvent event) {
+    try {
+      Message kickMessage = event.getKickMessage();
+      if (kickMessage != null) {
+        listener.handlePin(event.getBan().getPlayer(), kickMessage);
+      }
+    } catch (NoSuchMethodError ignored) {
+    }
+  }
+
   @EventHandler
   public void onReload(PluginReloadedEvent event) {
     plugin.getPlugin().setupConfigs();
diff --git a/bungee/src/main/java/me/confuser/banmanager/webenhancer/bungee/listeners/BanListener.java b/bungee/src/main/java/me/confuser/banmanager/webenhancer/bungee/listeners/BanListener.java
@@ -1,7 +1,9 @@
 package me.confuser.banmanager.webenhancer.bungee.listeners;
 
+import me.confuser.banmanager.bungee.api.events.PlayerBannedEvent;
 import me.confuser.banmanager.bungee.api.events.PlayerDeniedEvent;
 import me.confuser.banmanager.bungee.api.events.PluginReloadedEvent;
+import me.confuser.banmanager.common.util.Message;
 import me.confuser.banmanager.webenhancer.bungee.BungeePlugin;
 import net.md_5.bungee.api.plugin.Listener;
 import net.md_5.bungee.event.EventHandler;
@@ -21,6 +23,17 @@ public void onDeny(PlayerDeniedEvent event) {
     listener.handlePin(event.getPlayer(), event.getMessage());
   }
 
+  @EventHandler
+  public void onBanned(PlayerBannedEvent event) {
+    try {
+      Message kickMessage = event.getKickMessage();
+      if (kickMessage != null) {
+        listener.handlePin(event.getBan().getPlayer(), kickMessage);
+      }
+    } catch (NoSuchMethodError ignored) {
+    }
+  }
+
   @EventHandler
   public void onReload(PluginReloadedEvent event) {
     plugin.getPlugin().setupConfigs();
diff --git a/common/src/main/java/me/confuser/banmanager/webenhancer/common/runnables/ExpiresSync.java b/common/src/main/java/me/confuser/banmanager/webenhancer/common/runnables/ExpiresSync.java
@@ -1,21 +1,17 @@
 package me.confuser.banmanager.webenhancer.common.runnables;
 
 import me.confuser.banmanager.common.ormlite.stmt.DeleteBuilder;
-import me.confuser.banmanager.common.BanManagerPlugin;
-import me.confuser.banmanager.common.runnables.BmRunnable;
 import me.confuser.banmanager.common.util.DateUtils;
 import me.confuser.banmanager.webenhancer.common.WebEnhancerPlugin;
 import me.confuser.banmanager.webenhancer.common.data.PlayerPinData;
 import me.confuser.banmanager.webenhancer.common.storage.PlayerPinStorage;
 
 import java.sql.SQLException;
 
-public class ExpiresSync extends BmRunnable {
+public class ExpiresSync implements Runnable {
   private PlayerPinStorage pinStorage;
 
   public ExpiresSync(WebEnhancerPlugin plugin) {
-    super(BanManagerPlugin.getInstance(), "pinCheck");
-
     pinStorage = plugin.getPlayerPinStorage();
   }
 
diff --git a/e2e/platforms/bukkit/configs/banmanager/messages.yml b/e2e/platforms/bukkit/configs/banmanager/messages.yml
@@ -136,7 +136,7 @@ messages:
   ban:
     player:
       disallowed: '&6You have been banned from this server for &4[reason]&6. Your appeal pin is [pin]'
-      kick: '&6You have been banned permanently for &4[reason]'
+      kick: '&6You have been banned permanently for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been permanently banned by [actor] for &4[reason]'
     error:
@@ -149,7 +149,7 @@ messages:
   tempban:
     player:
       disallowed: '&6You have been temporarily banned from this server for &4[reason] \n&6It expires in [expires]. Your appeal pin is [pin]'
-      kick: '&6You have been temporarily banned for &4[reason]'
+      kick: '&6You have been temporarily banned for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been temporarily banned for [expires] by [actor] for &4[reason]'
 
diff --git a/e2e/platforms/bungee/configs/banmanager/messages.yml b/e2e/platforms/bungee/configs/banmanager/messages.yml
@@ -143,7 +143,7 @@ messages:
     player:
       disallowed: '&6You have been banned from this server for &4[reason]&6. Your
         appeal pin is [pin]'
-      kick: '&6You have been banned permanently for &4[reason]'
+      kick: '&6You have been banned permanently for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: yyyy-MM-dd HH:mm:ss
     notify: '&6[player] has been permanently banned by [actor] for &4[reason]'
     error:
@@ -155,7 +155,7 @@ messages:
     player:
       disallowed: '&6You have been temporarily banned from this server for &4[reason]
         \n&6It expires in [expires]. Your appeal pin is [pin]'
-      kick: '&6You have been temporarily banned for &4[reason]'
+      kick: '&6You have been temporarily banned for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: yyyy-MM-dd HH:mm:ss
     notify: '&6[player] has been temporarily banned for [expires] by [actor] for &4[reason]'
   tempbanall:
diff --git a/e2e/platforms/fabric/configs/banmanager/messages.yml b/e2e/platforms/fabric/configs/banmanager/messages.yml
@@ -142,7 +142,7 @@ messages:
   ban:
     player:
       disallowed: '&6You have been banned from this server for &4[reason]&6. Your appeal pin is [pin]'
-      kick: '&6You have been banned permanently for &4[reason]'
+      kick: '&6You have been banned permanently for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been permanently banned by [actor] for &4[reason]'
     error:
@@ -155,7 +155,7 @@ messages:
   tempban:
     player:
       disallowed: '&6You have been temporarily banned from this server for &4[reason] \n&6It expires in [expires]. Your appeal pin is [pin]'
-      kick: '&6You have been temporarily banned for &4[reason]'
+      kick: '&6You have been temporarily banned for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been temporarily banned for [expires] by [actor] for &4[reason]'
 
diff --git a/e2e/platforms/sponge/configs/banmanager/messages.yml b/e2e/platforms/sponge/configs/banmanager/messages.yml
@@ -136,7 +136,7 @@ messages:
   ban:
     player:
       disallowed: '&6You have been banned from this server for &4[reason]&6. Your appeal pin is [pin]'
-      kick: '&6You have been banned permanently for &4[reason]'
+      kick: '&6You have been banned permanently for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been permanently banned by [actor] for &4[reason]'
     error:
@@ -149,7 +149,7 @@ messages:
   tempban:
     player:
       disallowed: '&6You have been temporarily banned from this server for &4[reason] \n&6It expires in [expires]. Your appeal pin is [pin]'
-      kick: '&6You have been temporarily banned for &4[reason]'
+      kick: '&6You have been temporarily banned for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been temporarily banned for [expires] by [actor] for &4[reason]'
 
diff --git a/e2e/platforms/sponge7/configs/banmanager/messages.yml b/e2e/platforms/sponge7/configs/banmanager/messages.yml
@@ -136,7 +136,7 @@ messages:
   ban:
     player:
       disallowed: '&6You have been banned from this server for &4[reason]&6. Your appeal pin is [pin]'
-      kick: '&6You have been banned permanently for &4[reason]'
+      kick: '&6You have been banned permanently for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been permanently banned by [actor] for &4[reason]'
     error:
@@ -149,7 +149,7 @@ messages:
   tempban:
     player:
       disallowed: '&6You have been temporarily banned from this server for &4[reason] \n&6It expires in [expires]. Your appeal pin is [pin]'
-      kick: '&6You have been temporarily banned for &4[reason]'
+      kick: '&6You have been temporarily banned for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: 'yyyy-MM-dd HH:mm:ss'
     notify: '&6[player] has been temporarily banned for [expires] by [actor] for &4[reason]'
 
diff --git a/e2e/platforms/velocity/configs/banmanager/messages.yml b/e2e/platforms/velocity/configs/banmanager/messages.yml
@@ -143,7 +143,7 @@ messages:
     player:
       disallowed: '&6You have been banned from this server for &4[reason]&6. Your
         appeal pin is [pin]'
-      kick: '&6You have been banned permanently for &4[reason]'
+      kick: '&6You have been banned permanently for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: yyyy-MM-dd HH:mm:ss
     notify: '&6[player] has been permanently banned by [actor] for &4[reason]'
     error:
@@ -155,7 +155,7 @@ messages:
     player:
       disallowed: '&6You have been temporarily banned from this server for &4[reason]
         \n&6It expires in [expires]. Your appeal pin is [pin]'
-      kick: '&6You have been temporarily banned for &4[reason]'
+      kick: '&6You have been temporarily banned for &4[reason]&6. Your appeal pin is [pin]'
       dateTimeFormat: yyyy-MM-dd HH:mm:ss
     notify: '&6[player] has been temporarily banned for [expires] by [actor] for &4[reason]'
   tempbanall:
diff --git a/e2e/tests/src/denied-pin.test.ts b/e2e/tests/src/denied-pin.test.ts
@@ -1,44 +1,44 @@
 import { TestBot, createBot, sendCommand, disconnectRcon, sleep } from './helpers'
 
+afterAll(async () => {
+  await disconnectRcon()
+})
+
 describe('Denied Pin Placeholder', () => {
-  let bannedBot: TestBot | null = null
   const BANNED_USERNAME = 'DeniedPinPlayer'
-  const TEST_TIMEOUT_MS = 60000
+  const TEST_TIMEOUT_MS = 120000
 
   const expectDeniedConnection = async (): Promise<string> => {
     let lastError: Error | null = null
 
     // Ban/tempban enforcement can be async across worker threads; retry denied connect checks.
-    for (let attempt = 1; attempt <= 3; attempt++) {
+    // Use a short connect timeout (10s) so limbo connections (neither kicked nor spawned)
+    // don't burn the full 30s default, leaving room for more retries.
+    for (let attempt = 1; attempt <= 8; attempt++) {
+      const bot = new TestBot(BANNED_USERNAME)
       try {
-        bannedBot = await createBot(BANNED_USERNAME)
-        await bannedBot.disconnect()
-        bannedBot = null
+        await bot.connect(10000)
+        await bot.disconnect()
         lastError = new Error(`Attempt ${attempt}: player connected while expected to be denied`)
       } catch (error: unknown) {
         const errorMessage = error instanceof Error ? error.message : String(error)
         if (errorMessage.includes('was kicked')) {
           return errorMessage
         }
         lastError = error instanceof Error ? error : new Error(String(error))
+        try { await bot.disconnect() } catch (e) {}
       }
 
-      await sleep(1000)
+      await sleep(2000)
     }
 
     throw lastError ?? new Error('Expected denied connection but did not receive a denial kick')
   }
 
   afterEach(async () => {
-    try {
-      await sendCommand(`bmunban ${BANNED_USERNAME}`)
-    } catch (e) {}
-    await bannedBot?.disconnect()
-    bannedBot = null
-  })
-
-  afterAll(async () => {
-    await disconnectRcon()
+    try { await sendCommand(`bmunban ${BANNED_USERNAME}`) } catch (e) {}
+    try { await sendCommand(`bmuntempban ${BANNED_USERNAME}`) } catch (e) {}
+    await sleep(2000)
   })
 
   test('[pin] placeholder in ban message is replaced with actual pin', async () => {
@@ -64,7 +64,7 @@ describe('Denied Pin Placeholder', () => {
   test('[pin] placeholder in tempban message is replaced with actual pin', async () => {
     const tempbanResponse = await sendCommand(`bmtempban ${BANNED_USERNAME} 1h Testing tempban pin placeholder`)
     console.log(`Tempban response: ${tempbanResponse}`)
-    await sleep(2000)
+    await sleep(5000)
 
     const errorMessage = await expectDeniedConnection()
     console.log(`Player was denied (tempban) as expected: ${errorMessage}`)
@@ -82,3 +82,65 @@ describe('Denied Pin Placeholder', () => {
     }
   }, TEST_TIMEOUT_MS)
 })
+
+describe('Online Kick Pin Placeholder', () => {
+  const KICK_USERNAME = 'OnlineKickPin'
+  const TEST_TIMEOUT_MS = 60000
+  let bot: TestBot | null = null
+
+  afterEach(async () => {
+    await bot?.disconnect()
+    bot = null
+    try { await sendCommand(`bmunban ${KICK_USERNAME}`) } catch (e) {}
+    try { await sendCommand(`bmuntempban ${KICK_USERNAME}`) } catch (e) {}
+    await sleep(2000)
+  })
+
+  test('[pin] placeholder in kick message is replaced when banning an online player', async () => {
+    bot = await createBot(KICK_USERNAME)
+    const kickPromise = bot.waitForKick()
+
+    await sleep(1000)
+    const banResponse = await sendCommand(`bmban ${KICK_USERNAME} Testing online kick pin`)
+    console.log(`Ban response: ${banResponse}`)
+
+    const kickReason = await kickPromise
+    console.log(`Online player kicked with reason: ${kickReason}`)
+    bot = null
+
+    expect(kickReason).not.toContain('[pin]')
+
+    const pinMatch = kickReason.match(/pin is (\d{6})/)
+    expect(pinMatch).not.toBeNull()
+
+    if (pinMatch != null) {
+      const pin = pinMatch[1]
+      console.log(`Extracted pin from online kick message: ${pin}`)
+      expect(pin).toMatch(/^\d{6}$/)
+    }
+  }, TEST_TIMEOUT_MS)
+
+  test('[pin] placeholder in kick message is replaced when tempbanning an online player', async () => {
+    bot = await createBot(KICK_USERNAME)
+    const kickPromise = bot.waitForKick()
+
+    await sleep(1000)
+    const tempbanResponse = await sendCommand(`bmtempban ${KICK_USERNAME} 1h Testing online tempban kick pin`)
+    console.log(`Tempban response: ${tempbanResponse}`)
+
+    const kickReason = await kickPromise
+    console.log(`Online player kicked (tempban) with reason: ${kickReason}`)
+    bot = null
+
+    expect(kickReason).not.toContain('[pin]')
+
+    const pinMatch = kickReason.match(/pin is (\d{6})/)
+    expect(pinMatch).not.toBeNull()
+
+    if (pinMatch != null) {
+      const pin = pinMatch[1]
+      console.log(`Extracted pin from online tempban kick message: ${pin}`)
+      expect(pin).toMatch(/^\d{6}$/)
+    }
+  }, TEST_TIMEOUT_MS)
+})
diff --git a/e2e/tests/src/helpers/bot.ts b/e2e/tests/src/helpers/bot.ts
diff --git a/fabric/src/main/java/me/confuser/banmanager/webenhancer/fabric/listeners/EventListener.java b/fabric/src/main/java/me/confuser/banmanager/webenhancer/fabric/listeners/EventListener.java
diff --git a/gradle.properties b/gradle.properties
diff --git a/sponge-api7/src/main/java/me/confuser/banmanager/webenhancer/sponge/listeners/ReportListener.java b/sponge-api7/src/main/java/me/confuser/banmanager/webenhancer/sponge/listeners/ReportListener.java
diff --git a/sponge/src/main/java/me/confuser/banmanager/webenhancer/sponge/listeners/ReportListener.java b/sponge/src/main/java/me/confuser/banmanager/webenhancer/sponge/listeners/ReportListener.java
diff --git a/velocity/src/main/java/me/confuser/banmanager/webenhancer/velocity/listener/BanListener.java b/velocity/src/main/java/me/confuser/banmanager/webenhancer/velocity/listener/BanListener.java
