dispatchCommand not working for CommandAPI commands on 1.20.6+ Paper

[XLordalX]

CommandAPI version

9.5.1

Minecraft version

1.20

Are you shading the CommandAPI?

No

What I did

CommandAPICommand("shquests")
        .withSubcommand(
            CommandAPICommand("path")
                .withArguments(
                    EnumArgument("path", Path::class.java)
                )
                .executes(CommandExecutor { sender, args ->
                    val player = sender as Player

                    player.choosePath(theme, progressor, registry, args.getTyped("path"))
                }, ExecutorType.PLAYER)
        ).register();

// Sometime during runtime
Bukkit.dispatchCommand(me, 'shquests path TEST');

What actually happened

What should have happened

The command should have executed like it does when executed manually from a player (without dispatchCommand).

Server logs and CommandAPI config

No response

Other

No response

[XLordalX]

I digged through the Brigadier changes in paper 1.20.6 and found out there were some permission related changes. I'm not sure why this only applies to commands executed by dispatchCommand though.

For now I am able to work around this issue by setting the permission minecraft.commands.shquests (as if it's treated as a vanilla command).

[willkroboth]

When a command is run through dispatchCommand, the server tries to retrieve a Command instance from its CommandMap. Since Paper 1.20.6, due to https://github.com/PaperMC/Paper/pull/8235, the primary source of truth for commands is the Brigadier CommandDispatcher that contains CommandNode objects. So, to bridge this gap, Paper has a BukkitBrigForwardingMap that converts CommandNodes to Commands:

public class BukkitBrigForwardingMap extends HashMap<String, Command> {
    @Override
    public Command get(Object key) {
        CommandNode<?> node = this.getDispatcher().getRoot().getChild((String) key);
        if (node == null) {
            return null;
        }

        if (node instanceof BukkitCommandNode bukkitCommandNode) {
            // Bukkit commands end up here
            return bukkitCommandNode.getBukkitCommand();
        }

        return PaperBrigadier.wrapNode(node);
    }
}

public final class PaperBrigadier {
    public static Command wrapNode(CommandNode node) {
        if (!(node instanceof LiteralCommandNode)) {
            throw new IllegalArgumentException("Unsure how to wrap a " + node);
        }

        if (!(node instanceof PluginCommandNode pluginCommandNode)) {
            // Currently, CommandAPI commands end up here
            return new VanillaCommandWrapper(null, node);
        }
        CommandNode<CommandSourceStack> argumentCommandNode = node;
        if (argumentCommandNode.getRedirect() != null) {
            argumentCommandNode = argumentCommandNode.getRedirect();
        }

        Map<CommandNode<CommandSourceStack>, String> map = PaperCommands.INSTANCE.getDispatcherInternal().getSmartUsage(argumentCommandNode, DUMMY);
        String usage = map.isEmpty() ? pluginCommandNode.getUsageText() :  pluginCommandNode.getUsageText() + " " + String.join("\n" + pluginCommandNode.getUsageText() + " ", map.values());
        // Paper commands end up here
        return new PluginVanillaCommandWrapper(pluginCommandNode.getName(), pluginCommandNode.getDescription(), usage, pluginCommandNode.getAliases(), node, pluginCommandNode.getPlugin());
    }
}

public class PluginVanillaCommandWrapper extends VanillaCommandWrapper implements PluginIdentifiableCommand {
    public PluginVanillaCommandWrapper(String name, String description, String usageMessage, List<String> aliases, CommandNode<CommandSourceStack> vanillaCommand, Plugin plugin) {
        super(name, description, usageMessage, aliases, vanillaCommand);
        this.plugin = plugin;
        this.alises = aliases;
    }
}

public class VanillaCommandWrapper extends BukkitCommand {
    public VanillaCommandWrapper(String name, String description, String usageMessage, List<String> aliases, CommandNode<CommandSourceStack> vanillaCommand) {
        super(name, description, usageMessage, aliases);
        this.vanillaCommand = vanillaCommand;
    }

    public VanillaCommandWrapper(Commands dispatcher, CommandNode<CommandSourceStack> vanillaCommand) {
        super(vanillaCommand.getName(), "A Mojang provided command.", vanillaCommand.getUsageText(), Collections.EMPTY_LIST);
        this.vanillaCommand = vanillaCommand;
        // Incorrectly sets the permission to, for example, `minecraft.commands.shquests`
        this.setPermission(VanillaCommandWrapper.getPermission(vanillaCommand));
    }
}

Currently, CommandAPI commands end up being wrapped in VanillaCommandWrappers using the second constructor, which calls this.setPermission(VanillaCommandWrapper.getPermission(vanillaCommand)). Hence, when dispatchCommand tries to run the command, it thinks there is a permission (minecraft.commands.shquests in this case), even when one is not set on the CommandAPICommand.

Commands created through Paper's Brig API don't have this problem, because their CommandNodes are represented using the PluginCommandNode class. PaperBrigadier#wrapNode filters these out, and they end up using the first constructor in VanillaCommandWrapper, which doesn't call setPermission, so it defaults to null. This means that the permission check when executing the Bukkit-type command always passes, but the CommandNode's requirement can do permission checks later.

This is kind of the same underlying problem as https://github.com/JorelAli/CommandAPI/issues/578, where accessing CommandAPI commands through the CommandMap doesn't correctly represent their permission. To fix this issue, CommandAPI commands probably somehow need to emerge from BukkitBrigForwardingMap#get with either the correct permission (also fixing #578), or a null permission (so they act like Paper commands and can check the permission in the CommandNode's requirement).