fix(spigot): support ViaVersion 5.x legacy initializer unwrapping

AGENTS.md

@@ -77,6 +77,26 @@ curl -I -L --fail https://github.com/minekube/connect-java/releases/download/<ve
   guarded by `Libp2pRuntimeBoundaryTest`, and constructor alignment by
   `core/.../tunnel/p2p/Libp2pEndpointRuntimeInitTest`.
 
+## Third-party platform APIs (ViaVersion & friends)
+
+- Connect runs against a wide range of server/plugin versions, so an API that
+  drifts across majors must not be bound at compile time. Resolve cross-version
+  accessors reflectively by name (newest first) and degrade to skipping the
+  workaround with a warning when no known accessor exists; see
+  `SpigotInjector#unwrapViaInitializer` and `SpigotInjectorViaLegacyPathTest`.
+- Only plain Spigot/CraftBukkit takes Via's wrapping (legacy) injector path. On
+  Paper `BukkitViaInjector` registers a `ChannelInitializeListener` instead, which
+  Connect's local channel picks up for free — so Paper never exercises the unwrap.
+- Injector failures must stay diagnosable: `ConnectPlatform.enable()` catches
+  `Throwable` (not `Exception`) because reflective signature drift arrives as an
+  `Error`, making it a logged, orderly injection failure rather than an unhandled
+  `Error` escaping `onEnable()`. `SpigotPlatform.enable()` still disables the plugin
+  on a false return, so the value is the diagnosable log line, not continued operation.
+  Guarded by `core/.../ConnectPlatformEnableFailureContainmentTest`.
+- Compile-only platform deps live in `build-logic/.../Versions.kt` and are excluded
+  from the shaded jar by `provided(...)`; the `viaversion-bukkit` artifact declares
+  no transitive deps, so `viaversion-common` must be requested explicitly.
+
 ## Velocity Join Bugs
 
 - For Velocity proxy issues, test both `CONFIGURATION` and `PLAY` state packet

build-logic/src/main/kotlin/Versions.kt

@@ -32,7 +32,10 @@ object Versions {
     const val cloudVersion = "1.5.0"
     const val adventureApiVersion = "4.10.0"
     const val adventurePlatformVersion = "4.0.0"
-    const val viaVersionVersion = "4.0.0"
+    // Compile-only (platform-provided). Must stay on a 5.x: Via 5.0 renamed
+    // BukkitChannelInitializer#getOriginal() to original(), and SpigotInjector resolves both names
+    // reflectively so Via 4.x servers keep working. See SpigotInjectorViaLegacyPathTest.
+    const val viaVersionVersion = "5.11.0"
     const val gRPCVersion = "1.44.0"
     const val protocVersion = "3.19.4"
     const val bstatsVersion = "3.0.2"

core/src/main/java/com/minekube/connect/ConnectPlatform.java

@@ -133,8 +133,15 @@ public boolean enable(Module... postInitializeModules) {
                 logger.error("Failed to inject the packet listener!");
                 return false;
             }
-        } catch (Exception exception) {
-            logger.error("Failed to inject the packet listener!", exception);
+        } catch (Throwable throwable) {
+            // Platform injectors reflect against server/plugin internals, so signature drift (e.g.
+            // a method removed in a new ViaVersion major) arrives as an Error such as
+            // NoSuchMethodError rather than an Exception. Catching Throwable here logs it through
+            // Connect's logger and reports a normal injection failure instead of letting an
+            // unhandled Error escape onEnable(). A false return may still make the platform
+            // disable the plugin—SpigotPlatform.enable() does—so this buys an orderly,
+            // diagnosable failure, not continued operation.
+            logger.error("Failed to inject the packet listener!", throwable);
             return false;
         }
 

core/src/test/java/com/minekube/connect/ConnectPlatformEnableFailureContainmentTest.java

@@ -0,0 +1,57 @@
+package com.minekube.connect;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.google.inject.Injector;
+import com.minekube.connect.api.ConnectApi;
+import com.minekube.connect.api.inject.PlatformInjector;
+import com.minekube.connect.api.logger.ConnectLogger;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Guards that a failing platform injector is reported and logged as a failed enable result rather
+ * than letting its {@link Error} propagate out of {@code enable()}.
+ *
+ * <p>Platform injectors reflect against server and plugin internals (NMS, ViaVersion, ProtocolLib),
+ * so signature drift arrives as an {@link Error} - the ViaVersion 5.x
+ * {@code BukkitChannelInitializer.getOriginal()} removal produced a {@link NoSuchMethodError} (see
+ * {@code SpigotInjectorViaLegacyPathTest}). {@code enable()} used to catch only {@link Exception},
+ * so that {@code Error} escaped {@code onEnable()} and the platform disabled the plugin outright.
+ */
+class ConnectPlatformEnableFailureContainmentTest {
+
+    @Test
+    void enableContainsAnInjectorError() throws Exception {
+        PlatformInjector injector = mock(PlatformInjector.class);
+        NoSuchMethodError error = new NoSuchMethodError(
+                "io.netty.channel.ChannelInitializer "
+                        + "com.viaversion.viaversion.bukkit.handlers.BukkitChannelInitializer"
+                        + ".getOriginal()");
+        when(injector.inject()).thenThrow(error);
+        ConnectLogger logger = mock(ConnectLogger.class);
+
+        assertFalse(platform(injector, logger).enable(),
+                "an injector Error must be reported and logged as a failed injection, with "
+                        + "enable() returning false instead of allowing it to propagate");
+        verify(logger).error("Failed to inject the packet listener!", error);
+    }
+
+    /** An injector that merely returns false is still reported the same way. */
+    @Test
+    void enableReportsAFailedInjection() throws Exception {
+        PlatformInjector injector = mock(PlatformInjector.class);
+        when(injector.inject()).thenReturn(false);
+
+        assertFalse(platform(injector, mock(ConnectLogger.class)).enable());
+    }
+
+    private static ConnectPlatform platform(PlatformInjector injector, ConnectLogger logger) {
+        // No admission coordinator: enable() never touches it, and a real one would leak its
+        // cleanup executor because these tests never reach disable().
+        return new ConnectPlatform(
+                mock(ConnectApi.class), injector, logger, mock(Injector.class), null);
+    }
+}

spigot/build.gradle.kts

@@ -20,6 +20,12 @@ dependencies {
     testImplementation(testFixtures(projects.core))
     testImplementation("com.mojang", "authlib", authlibVersion)
     testImplementation("io.netty", "netty-transport", Versions.nettyVersion)
+    // Real ViaVersion 5.x on the test classpath so SpigotInjectorViaLegacyPathTest exercises the
+    // legacy (non-Paper) injector unwrap against the actual BukkitChannelInitializer API.
+    // viaversion-bukkit publishes no transitive deps, so viaversion-common (which holds the
+    // ViaChannelInitializer superclass declaring original()) has to be requested explicitly.
+    testImplementation("com.viaversion", "viaversion-bukkit", Versions.viaVersionVersion)
+    testImplementation("com.viaversion", "viaversion-common", Versions.viaVersionVersion)
     testImplementation("dev.folia", "folia-api", Versions.spigotVersion) {
         attributes {
             attribute(TargetJvmVersion.TARGET_JVM_VERSION_ATTRIBUTE, 17)
@@ -51,3 +57,6 @@ relocate("org.yaml")
 provided("com.mojang", "authlib", authlibVersion)
 provided("com.google.code.gson", "gson", gsonVersion)
 provided("com.viaversion", "viaversion-bukkit", Versions.viaVersionVersion)
+// Since Via 5.0 BukkitChannelInitializer extends ViaChannelInitializer, which lives here, so javac
+// needs it on the compile classpath to resolve the type. Provided by the ViaVersion plugin jar.
+provided("com.viaversion", "viaversion-common", Versions.viaVersionVersion)

spigot/src/main/java/com/minekube/connect/inject/spigot/SpigotInjector.java

@@ -57,6 +57,14 @@
 import org.checkerframework.checker.nullness.qual.NonNull;
 
 public final class SpigotInjector extends CommonPlatformInjector {
+    /**
+     * Names ViaVersion has used for the accessor returning the wrapped, original channel
+     * initializer, newest first: {@code original()} since Via 5.0, {@code getOriginal()} on Via 4.x.
+     *
+     * @see #unwrapViaInitializer(ChannelInitializer)
+     */
+    private static final String[] VIA_ORIGINAL_ACCESSORS = {"original", "getOriginal"};
+
     private final ConnectLogger logger;
     private final BedrockIdentityEnforcer bedrockIdentityEnforcer;
     private final String packetHandlerName;
@@ -303,7 +311,7 @@ private ChannelInitializer<Channel> getChildHandler(ChannelFuture listeningChann
                 childHandler = (ChannelInitializer<Channel>) childHandlerField.get(handler);
                 // ViaVersion non-Paper-injector workaround so we aren't double-injecting
                 if (isViaVersion && childHandler instanceof BukkitChannelInitializer) {
-                    childHandler = ((BukkitChannelInitializer) childHandler).getOriginal();
+                    childHandler = unwrapViaInitializer(childHandler);
                 }
                 break;
             } catch (Exception e) {
@@ -319,6 +327,52 @@ private ChannelInitializer<Channel> getChildHandler(ChannelFuture listeningChann
         return childHandler;
     }
 
+    /**
+     * Unwraps ViaVersion's legacy (non-Paper) channel initializer so we initialize the server's own
+     * initializer instead of Via's wrapper, which would double-inject Via's handlers.
+     *
+     * <p>The accessor was renamed across Via majors: {@code getOriginal()} on Via 4.x, {@code
+     * original()} since Via 5.0 (pulled up into {@code
+     * com.viaversion.viaversion.platform.WrappedChannelInitializer}). Connect supports both, so it
+     * is resolved reflectively by name instead of being bound at compile time.
+     *
+     * <p>Every failure degrades to "unwrap skipped" instead of propagating. With reflective lookup,
+     * a missing accessor name raises {@link NoSuchMethodException} and is skipped in favour of the
+     * next known name; if none match, the wrapper is returned with a warning.
+     */
+    @SuppressWarnings("unchecked")
+    private ChannelInitializer<Channel> unwrapViaInitializer(
+            ChannelInitializer<Channel> viaInitializer) {
+        for (String accessor : VIA_ORIGINAL_ACCESSORS) {
+            Method method;
+            try {
+                method = viaInitializer.getClass().getMethod(accessor);
+            } catch (NoSuchMethodException e) {
+                continue; // older/newer Via, try the next known name
+            }
+            try {
+                method.setAccessible(true);
+                Object original = method.invoke(viaInitializer);
+                if (original instanceof ChannelInitializer) {
+                    return (ChannelInitializer<Channel>) original;
+                }
+                logger.warn(accessor + "() on " + viaInitializer.getClass().getName()
+                        + " returned " + original + " instead of a ChannelInitializer, "
+                        + "continuing without unwrapping it.");
+                return viaInitializer;
+            } catch (Throwable throwable) {
+                logger.warn("Failed to unwrap ViaVersion's channel initializer via "
+                        + accessor + "(), continuing without unwrapping it: " + throwable);
+                return viaInitializer;
+            }
+        }
+        logger.warn("Could not unwrap ViaVersion's channel initializer: none of "
+                + String.join("(), ", VIA_ORIGINAL_ACCESSORS) + "() exists on "
+                + viaInitializer.getClass().getName() + ". Continuing without unwrapping it, "
+                + "please report this with your ViaVersion version.");
+        return viaInitializer;
+    }
+
     @Override
     public void shutdown() {
         if (this.allServerChannels != null) {

spigot/src/main/java/com/minekube/connect/module/SpigotPlatformModule.java

@@ -109,7 +109,8 @@ public CommonPlatformInjector platformInjector(
         final String VIAVERSION_DOWNLOAD_URL = "https://ci.viaversion.com/job/ViaVersion/";
         boolean isViaVersion = Bukkit.getPluginManager().getPlugin("ViaVersion") != null;
         if (isViaVersion) {
-            // Ensure that we have the latest 4.0.0 changes and not an older ViaVersion version
+            // Ensure that ViaVersion exposes the API introduced in 4.0.0, rather than an older
+            // version. The injector resolves later ViaVersion accessor changes reflectively.
             if (getClassSilently("com.viaversion.viaversion.api.ViaManager") == null) {
                 logger.warn("The plugin version of ViaVersion is too old, " +
                                 "please install the latest release from {}",