feat(modules): load/initialize split and guarded module commands

AGENTS.md

@@ -59,6 +59,13 @@ outcome that cannot occur.
   class loader, so a module must clean up its own partial registrations before throwing (documented
   on `PluginModule#initialize`). `SimplePluginModule` tracks listeners but unregisters nothing by
   itself. Changing this is a contract change for every module, not a bug fix.
+- **Module commands are declared, never unregistered.** Paper only accepts commands during its
+  lifecycle commands event, before modules initialize, and its API cannot remove them. A module
+  overrides `PluginModule#declareCommands` (a default no-op), which
+  `ModuleLifecycleManager#declareCommands` calls between `load()` and `initializeLoaded()`.
+  `CommandGuard` rebuilds each tree so executors and suggestions run only while the module is
+  active, and the host's `ModuleUnavailableHandler` replies otherwise. Redirects are rejected
+  because they would bypass the guard. A module never cleans up its commands.
 - **`ModuleLifecycleManager` is not thread-safe.** Its state must be touched from one thread (the
   server main thread on Paper).
 - **`SchemaMigrator` is abstract and dialect-agnostic.** It provides no SQL of its own — subclasses

README.md

@@ -49,7 +49,19 @@ by a `ModuleManifest` and loaded at runtime by its own `URLClassLoader`.
   (configurate-serializable).
 - `ModuleLoader` — reads module jars from a directory and instantiates their entry classes.
 - `ModuleLifecycleManager<T>` — owns the runtime lifecycle: `start()`, `stop()`,
-  `reloadAsync(moduleName)`, `getActiveModules()`.
+  `reloadAsync(moduleName)`, `getActiveModules()`. `start()` is `load()` then `initializeLoaded()`;
+  call the two separately when the host must do something with the loaded modules first, such as
+  letting them declare commands while the server still accepts registrations.
+- `PluginModule#declareCommands(ModuleCommandRoot, ModuleCommandRegistrar)` — a default no-op that a
+  module overrides to add Brigadier commands: subcommands under the host's root, or top-level
+  commands. Both parameters guard every tree.
+
+> **Commands:** Paper only accepts commands during `LifecycleEvents.COMMANDS`, before modules
+> initialize, and cannot unregister them. Call `declareCommands(commands, root, unavailable)` from that
+> event handler, every time it fires, between `load()` and `initializeLoaded()`. A module command runs
+> module code only while its module is active; when the module failed to initialize or has stopped,
+> the host's `ModuleUnavailableHandler` replies instead. Commands therefore need no cleanup on a
+> failed start. Module commands must not use redirects.
 
 > **Failed start:** if `initialize` throws, the manager closes the module's class loader but does
 > **not** call `shutdown`. A module must do its fallible work before registering anything, or undo
@@ -69,7 +81,14 @@ Path moduleDir = getDataFolder().toPath().resolve("modules");
 ModuleLoader loader = new ModuleLoader(moduleDir);
 var lifecycle = new ModuleLifecycleManager<>(this, loader, MyPlugin.class.getName(), getLogger());
 
-lifecycle.start();                 // load + initialize all modules (call from the main thread)
+lifecycle.load();                  // load module jars, no initialize yet
+getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS, event -> {
+    LiteralArgumentBuilder<CommandSourceStack> root = Commands.literal("myplugin");
+    lifecycle.declareCommands(event.registrar(), root,
+            (source, module) -> source.getSender().sendMessage("That feature is unavailable."));
+    event.registrar().register(root.build());
+});
+lifecycle.initializeLoaded();      // initialize (call from the main thread)
 // ...
 lifecycle.reloadAsync("economy"); // refresh one reloadable module's config
 // ...

src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java

@@ -0,0 +1,73 @@
+package com.minecraftcitiesnetwork.pluginInfrastructure.modules;
+
+import com.mojang.brigadier.Command;
+import com.mojang.brigadier.builder.ArgumentBuilder;
+import com.mojang.brigadier.builder.LiteralArgumentBuilder;
+import com.mojang.brigadier.builder.RequiredArgumentBuilder;
+import com.mojang.brigadier.suggestion.SuggestionProvider;
+import com.mojang.brigadier.tree.ArgumentCommandNode;
+import com.mojang.brigadier.tree.CommandNode;
+import com.mojang.brigadier.tree.LiteralCommandNode;
+import io.papermc.paper.command.brigadier.CommandSourceStack;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.function.BooleanSupplier;
+
+/**
+ * Rebuilds a module's command tree so that every executor and suggestion provider first checks
+ * whether the module is active. Brigadier nodes are immutable once built, so the guard is applied
+ * by rebuilding each node from {@link CommandNode#createBuilder()}.
+ */
+final class CommandGuard {
+
+    private CommandGuard() {
+    }
+
+    // Rebuilding a literal yields a literal, and an argument node's builder is a RequiredArgumentBuilder.
+    @SuppressWarnings("unchecked")
+    static @NotNull LiteralCommandNode<CommandSourceStack> guard(
+            @NotNull LiteralArgumentBuilder<CommandSourceStack> command,
+            @NotNull String moduleName,
+            @NotNull BooleanSupplier active,
+            @NotNull ModuleUnavailableHandler unavailable
+    ) {
+        return (LiteralCommandNode<CommandSourceStack>) rebuild(command.build(), moduleName, active, unavailable);
+    }
+
+    @SuppressWarnings("unchecked")
+    private static @NotNull CommandNode<CommandSourceStack> rebuild(
+            @NotNull CommandNode<CommandSourceStack> node,
+            @NotNull String moduleName,
+            @NotNull BooleanSupplier active,
+            @NotNull ModuleUnavailableHandler unavailable
+    ) {
+        // A redirect points at a node outside this rebuild, which would bypass the guard.
+        if (node.getRedirect() != null) {
+            throw new IllegalArgumentException("Module '" + moduleName + "' declared a redirect at '"
+                    + node.getName() + "'; module commands must not use redirects");
+        }
+        ArgumentBuilder<CommandSourceStack, ?> builder = node.createBuilder();
+        Command<CommandSourceStack> command = node.getCommand();
+        if (command != null) {
+            builder.executes(ctx -> {
+                if (active.getAsBoolean()) {
+                    return command.run(ctx);
+                }
+                unavailable.handle(ctx.getSource(), moduleName);
+                return 0;
+            });
+        }
+        if (node instanceof ArgumentCommandNode<CommandSourceStack, ?> argumentNode
+                && argumentNode.getCustomSuggestions() != null) {
+            SuggestionProvider<CommandSourceStack> suggestions = argumentNode.getCustomSuggestions();
+            RequiredArgumentBuilder<CommandSourceStack, ?> argument = (RequiredArgumentBuilder<CommandSourceStack, ?>) builder;
+            argument.suggests((ctx, suggestionsBuilder) -> active.getAsBoolean()
+                    ? suggestions.getSuggestions(ctx, suggestionsBuilder)
+                    : suggestionsBuilder.buildFuture());
+        }
+        for (CommandNode<CommandSourceStack> child : node.getChildren()) {
+            builder.then(rebuild(child, moduleName, active, unavailable));
+        }
+        return builder.build();
+    }
+}

src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java

@@ -0,0 +1,32 @@
+package com.minecraftcitiesnetwork.pluginInfrastructure.modules;
+
+import com.mojang.brigadier.builder.LiteralArgumentBuilder;
+import io.papermc.paper.command.brigadier.CommandSourceStack;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Collection;
+import java.util.List;
+
+/**
+ * Registers top-level commands for {@link PluginModule#declareCommands}. Each tree is guarded by the
+ * module's availability before it reaches Paper's registrar, so a module cannot register an
+ * unguarded command.
+ */
+public interface ModuleCommandRegistrar {
+
+    /**
+     * Registers a top-level command. The tree must not contain redirects.
+     *
+     * @param command     the command tree
+     * @param description the help description, or {@code null}
+     * @param aliases     extra labels
+     */
+    void register(@NotNull LiteralArgumentBuilder<CommandSourceStack> command,
+                  @Nullable String description, @NotNull Collection<String> aliases);
+
+    default void register(@NotNull LiteralArgumentBuilder<CommandSourceStack> command,
+                          @Nullable String description) {
+        register(command, description, List.of());
+    }
+}

src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java

@@ -0,0 +1,15 @@
+package com.minecraftcitiesnetwork.pluginInfrastructure.modules;
+
+import com.mojang.brigadier.builder.LiteralArgumentBuilder;
+import io.papermc.paper.command.brigadier.CommandSourceStack;
+import org.jetbrains.annotations.NotNull;
+
+/** The host's root command, as offered to {@link PluginModule#declareCommands}. */
+public interface ModuleCommandRoot {
+
+    /**
+     * Adds a subcommand under the host's root command. The tree is guarded by the module's
+     * availability; it must not contain redirects.
+     */
+    void then(@NotNull LiteralArgumentBuilder<CommandSourceStack> subcommand);
+}

src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java

@@ -1,15 +1,26 @@
 package com.minecraftcitiesnetwork.pluginInfrastructure.modules;
 
+import com.mojang.brigadier.builder.LiteralArgumentBuilder;
+import com.mojang.brigadier.tree.LiteralCommandNode;
+import io.papermc.paper.command.brigadier.CommandSourceStack;
+import io.papermc.paper.command.brigadier.Commands;
 import org.bukkit.plugin.Plugin;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BooleanSupplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -38,6 +49,28 @@ public class ModuleLifecycleManager<T extends Plugin> {
      */
     private final Map<String, LoadedModule<T>> activeModules = new LinkedHashMap<>();
 
+    /**
+     * Modules returned by {@link #load()} and not yet passed to {@link #initializeLoaded()}, or
+     * {@code null} when nothing is waiting.
+     */
+    private List<LoadedModule<T>> loadedModules;
+
+    /**
+     * Names of the active modules, read by command guards. Commands can run off the main thread,
+     * so this is concurrent while the rest of the manager is not.
+     */
+    private final Set<String> activeNames = ConcurrentHashMap.newKeySet();
+
+    /**
+     * Guarded command trees keyed by module name. Kept after a module stops so that a later
+     * commands event (Paper re-fires it on {@code /minecraft:reload}) registers the same commands,
+     * which then reply as unavailable.
+     */
+    private final Map<String, DeclaredCommands> declaredCommands = new LinkedHashMap<>();
+
+    /** Modules from the current {@link #load()} that have already declared their commands. */
+    private final Set<String> declaredThisLoad = new HashSet<>();
+
     /**
      * Creates a manager that expects modules targeting the host plugin's own class name and logs
      * through the plugin's logger.
@@ -59,27 +92,123 @@ public ModuleLifecycleManager(
     }
 
     /**
-     * Loads every module from disk and initializes it against the host plugin.
+     * Loads every module from disk and initializes it against the host plugin. Equivalent to
+     * {@link #load()} followed by {@link #initializeLoaded()}.
      *
-     * @throws IllegalStateException if modules are already active; call {@link #stop()} first
+     * @throws IllegalStateException if modules are already loaded or active; call {@link #stop()} first
      * @throws IOException           if the module directory cannot be scanned
      */
     public void start() throws IOException {
-        if (!this.activeModules.isEmpty()) {
-            throw new IllegalStateException("Modules already started; call stop() or reload() first");
+        load();
+        initializeLoaded();
+    }
+
+    /**
+     * Loads every module from disk without initializing any of them. The host can then inspect the
+     * returned modules, for example to let them declare commands while the server still accepts
+     * registrations, before calling {@link #initializeLoaded()}.
+     *
+     * <p>A loaded module's classes are live but {@link PluginModule#initialize} has not run, so the
+     * host must not expect any module state yet.
+     *
+     * @return the loaded modules, in load order
+     * @throws IllegalStateException if modules are already loaded or active; call {@link #stop()} first
+     * @throws IOException           if the module directory cannot be scanned
+     */
+    public @NotNull List<LoadedModule<T>> load() throws IOException {
+        if (this.loadedModules != null || !this.activeModules.isEmpty()) {
+            throw new IllegalStateException("Modules already loaded; call stop() first");
         }
-        List<LoadedModule<T>> loaded = this.moduleLoader.loadModulesFromDisk(this.expectedPluginClass,
-                this.logger);
+        List<LoadedModule<T>> loaded = new ArrayList<>(
+                this.moduleLoader.loadModulesFromDisk(this.expectedPluginClass, this.logger));
+        this.loadedModules = loaded;
+        this.declaredThisLoad.clear();
+        return List.copyOf(loaded);
+    }
+
+    /**
+     * Initializes the modules returned by the last {@link #load()}. A module whose
+     * {@code initialize} throws is unloaded and is not active afterwards; see
+     * {@link PluginModule#initialize} for its cleanup contract.
+     *
+     * @throws IllegalStateException if nothing is loaded
+     */
+    public void initializeLoaded() {
+        List<LoadedModule<T>> loaded = this.loadedModules;
+        if (loaded == null) {
+            throw new IllegalStateException("No modules loaded; call load() first");
+        }
+        this.loadedModules = null;
         for (LoadedModule<T> module : loaded) {
             initialize(module);
         }
     }
 
+    /**
+     * Registers module commands. Call it from Paper's {@code LifecycleEvents.COMMANDS} handler,
+     * after {@link #load()} and before {@link #initializeLoaded()}, every time the event fires.
+     *
+     * <p>Each loaded module that has not declared yet is asked to, via {@link PluginModule#declareCommands}.
+     * A module whose {@code declareCommands} throws is logged, unloaded and not initialized; nothing
+     * it declared is registered. Then every module's guarded commands are added: subcommands to
+     * {@code root}, top-level commands to {@code commands}. A guarded command runs the module's code
+     * only while the module is active; otherwise {@code unavailable} replies.
+     *
+     * @param commands    Paper's registrar from the commands event
+     * @param root        the host's root command builder; the host registers it after this call
+     * @param unavailable replies when a command's module is not active
+     */
+    public void declareCommands(
+            @NotNull Commands commands,
+            @NotNull LiteralArgumentBuilder<CommandSourceStack> root,
+            @NotNull ModuleUnavailableHandler unavailable
+    ) {
+        List<LoadedModule<T>> pending = this.loadedModules;
+        if (pending != null) {
+            Iterator<LoadedModule<T>> modules = pending.iterator();
+            while (modules.hasNext()) {
+                LoadedModule<T> loaded = modules.next();
+                String name = loaded.manifest().moduleName();
+                // A second copy with the same name is dropped by initializeLoaded and declares nothing.
+                if (!this.declaredThisLoad.add(name)) {
+                    continue;
+                }
+                BooleanSupplier active = () -> this.activeNames.contains(name);
+                DeclaredCommands declared = new DeclaredCommands(new ArrayList<>(), new ArrayList<>());
+                try {
+                    loaded.module().declareCommands(
+                            sub -> declared.subcommands().add(CommandGuard.guard(sub, name, active, unavailable)),
+                            (command, description, aliases) -> declared.topLevel().add(new TopLevelCommand(
+                                    CommandGuard.guard(command, name, active, unavailable), description,
+                                    List.copyOf(aliases))));
+                } catch (RuntimeException e) {
+                    this.logger.log(Level.SEVERE, "Failed to declare commands for module " + name + "; unloading", e);
+                    modules.remove();
+                    closeQuietly(loaded);
+                    continue;
+                }
+                this.declaredCommands.put(name, declared);
+            }
+        }
+        for (DeclaredCommands declared : this.declaredCommands.values()) {
+            declared.subcommands().forEach(root::then);
+            for (TopLevelCommand command : declared.topLevel()) {
+                commands.register(command.node(), command.description(), command.aliases());
+            }
+        }
+    }
+
     /**
      * Shuts down every active module in reverse initialization order and releases their class
-     * loaders. Safe to call when nothing is active.
+     * loaders. Modules that were loaded but never initialized are closed without
+     * {@link PluginModule#shutdown}. Safe to call when nothing is loaded or active.
      */
     public void stop() {
+        List<LoadedModule<T>> pending = this.loadedModules;
+        if (pending != null) {
+            this.loadedModules = null;
+            pending.forEach(this::closeQuietly);
+        }
         List<LoadedModule<T>> ordered = new ArrayList<>(this.activeModules.values());
         for (int i = ordered.size() - 1; i >= 0; i--) {
             shutdown(ordered.get(i));
@@ -132,6 +261,7 @@ private void initialize(@NotNull LoadedModule<T> loaded) {
         try {
             loaded.module().initialize(this.plugin, loaded.dataFolder());
             this.activeModules.put(name, loaded);
+            this.activeNames.add(name);
             this.logger.info("Initialized module " + name);
         } catch (ModuleInitializationException | RuntimeException e) {
             this.logger.log(Level.SEVERE, "Failed to initialize module " + name + "; unloading", e);
@@ -141,6 +271,7 @@ private void initialize(@NotNull LoadedModule<T> loaded) {
 
     private void shutdown(@NotNull LoadedModule<T> loaded) {
         String name = loaded.manifest().moduleName();
+        this.activeNames.remove(name);
         try {
             loaded.module().shutdown(this.plugin);
             this.logger.info("Shut down module " + name);
@@ -159,4 +290,17 @@ private void closeQuietly(@NotNull LoadedModule<T> loaded) {
                     "Failed to close class loader for module " + loaded.manifest().moduleName(), e);
         }
     }
+
+    private record DeclaredCommands(
+            @NotNull List<LiteralCommandNode<CommandSourceStack>> subcommands,
+            @NotNull List<TopLevelCommand> topLevel
+    ) {
+    }
+
+    private record TopLevelCommand(
+            @NotNull LiteralCommandNode<CommandSourceStack> node,
+            @Nullable String description,
+            @NotNull Collection<String> aliases
+    ) {
+    }
 }