From 170b3bbe81e4d4b79cec822927802edc15487245 Mon Sep 17 00:00:00 2001 From: bobhenl <18662086+bobhenl@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:12:25 +0100 Subject: [PATCH 1/2] fix: fire the command event for finished prompt commands Plugins that watch typed commands (alerts, logs) now see commands that went through a prompt. --- .../dispatch/CommandEventBridge.java | 40 ++++++++++ .../PaperImmediateActionDispatcher.java | 3 +- .../PaperPrimaryCommandDispatcher.java | 6 +- .../listener/PlayerCommandListener.java | 4 + .../PaperPrimaryCommandDispatcherTest.java | 74 +++++++++++++++++++ .../listener/PlayerCommandListenerTest.java | 15 ++++ 6 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java new file mode 100644 index 0000000..5349d02 --- /dev/null +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java @@ -0,0 +1,40 @@ +package dev.cyr1en.promptpaper.execution.dispatch; + +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; +import org.bukkit.event.player.PlayerCommandPreprocessEvent; + +/** + * Fires a {@link PlayerCommandPreprocessEvent} for a finished prompt command before it is + * dispatched, so plugins that watch typed commands (alerts, logs, aliases) also see commands that + * went through a prompt. The original event was cancelled when the prompt opened. + */ +public final class CommandEventBridge { + + private static final ThreadLocal FIRING = ThreadLocal.withInitial(() -> false); + + private CommandEventBridge() {} + + /** True while the bridge fires its own event, so the prompt listener can skip it. */ + public static boolean isFiring() { + return FIRING.get(); + } + + /** + * Fires the event and returns the command to run (without the leading slash), or null when a + * listener cancelled it. + */ + public static String fire(Player player, String command) { + PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(player, "/" + command); + FIRING.set(true); + try { + Bukkit.getPluginManager().callEvent(event); + } finally { + FIRING.set(false); + } + if (event.isCancelled()) { + return null; + } + return DispatchSanitizer.stripLeadingSlash(event.getMessage()); + } +} diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java index 08441ce..750622f 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java @@ -92,7 +92,8 @@ private void dispatchPlayer( initiator.execute( () -> { try { - boolean ok = Bukkit.dispatchCommand(player, command); + String finalCommand = CommandEventBridge.fire(player, command); + boolean ok = finalCommand == null || Bukkit.dispatchCommand(player, finalCommand); if (ok) { callback.onComplete(DispatchOutcome.success()); } else { diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java index bf8aca0..0dbfff7 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java @@ -80,7 +80,8 @@ private void dispatchPlayer( initiator.execute( () -> { try { - boolean ok = Bukkit.dispatchCommand(player, command); + String finalCommand = CommandEventBridge.fire(player, command); + boolean ok = finalCommand == null || Bukkit.dispatchCommand(player, finalCommand); if (ok) { callback.onComplete(DispatchOutcome.success()); } else { @@ -182,7 +183,8 @@ private void dispatchAttachment( } attachment.getPermissible().recalculatePermissions(); - boolean ok = Bukkit.dispatchCommand(player, command); + String finalCommand = CommandEventBridge.fire(player, command); + boolean ok = finalCommand == null || Bukkit.dispatchCommand(player, finalCommand); if (ok) { outcome = DispatchOutcome.success(); } else { diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java index f05e7b6..4133073 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java @@ -52,6 +52,10 @@ public void onPlayerCommand(PlayerCommandPreprocessEvent event) { plugin.getPluginLogger().debug("Command already cancelled, skipping"); return; } + if (dev.cyr1en.promptpaper.execution.dispatch.CommandEventBridge.isFiring()) { + // Finished prompt command, re-fired for other plugins. Never prompt it again. + return; + } var player = event.getPlayer(); var message = event.getMessage(); var commandName = diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java index 435c1f8..c58fca3 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java @@ -49,6 +49,80 @@ public boolean execute( server.getCommandMap().register(name, cmd); } + private Command recordingCommand(String name, List calls) { + Command cmd = + new Command(name) { + @Override + public boolean execute( + @NotNull CommandSender sender, @NotNull String commandLabel, @NotNull String[] args) { + calls.add(commandLabel + " " + String.join(" ", args)); + return true; + } + }; + server.getCommandMap().register(name, cmd); + return cmd; + } + + @Test + @DisplayName("PLAYER mode: fires PlayerCommandPreprocessEvent with the finished command") + void firesPreprocessEventBeforeDispatch() { + var player = createPlayer("EventPlayer"); + var calls = new java.util.ArrayList(); + recordingCommand("eventcmd", calls); + var seen = new java.util.ArrayList(); + server + .getPluginManager() + .registerEvents( + new org.bukkit.event.Listener() { + @org.bukkit.event.EventHandler + public void on(org.bukkit.event.player.PlayerCommandPreprocessEvent e) { + seen.add(e.getMessage()); + assertTrue(CommandEventBridge.isFiring()); + } + }, + plugin); + + var outcomeRef = new AtomicReference(); + dispatcher.dispatch(PrimaryDispatchRequest.player(player, "/eventcmd a b"), outcomeRef::set); + + assertEquals(List.of("/eventcmd a b"), seen); + assertEquals(List.of("eventcmd a b"), calls); + assertTrue(outcomeRef.get().isSuccess()); + assertFalse(CommandEventBridge.isFiring()); + } + + @Test + @DisplayName("PLAYER mode: a listener can rewrite or cancel the finished command") + void preprocessListenerCanRewriteOrCancel() { + var player = createPlayer("RewritePlayer"); + var calls = new java.util.ArrayList(); + recordingCommand("rewritten", calls); + recordingCommand("blockedcmd", calls); + server + .getPluginManager() + .registerEvents( + new org.bukkit.event.Listener() { + @org.bukkit.event.EventHandler + public void on(org.bukkit.event.player.PlayerCommandPreprocessEvent e) { + if (e.getMessage().startsWith("/original")) { + e.setMessage("/rewritten x"); + } else if (e.getMessage().startsWith("/blockedcmd")) { + e.setCancelled(true); + } + } + }, + plugin); + + var first = new AtomicReference(); + dispatcher.dispatch(PrimaryDispatchRequest.player(player, "/original y"), first::set); + var second = new AtomicReference(); + dispatcher.dispatch(PrimaryDispatchRequest.player(player, "/blockedcmd"), second::set); + + assertEquals(List.of("rewritten x"), calls); + assertTrue(first.get().isSuccess()); + assertTrue(second.get().isSuccess(), "a cancelled command is handled, like a typed one"); + } + @Test @DisplayName("PLAYER mode: executes command as player and invokes callback once with success") void dispatchesPlayerCommandSuccessfully() { diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/listener/PlayerCommandListenerTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/listener/PlayerCommandListenerTest.java index b9271b2..020cb32 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/listener/PlayerCommandListenerTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/listener/PlayerCommandListenerTest.java @@ -61,6 +61,21 @@ void commandInterceptionCancelsEventWhenScreenActive() { verify(screenManager).startSession(eq(player), eq("some command")); } + @Test + void refiredFinishedCommandIsNeverPromptedAgain() { + when(engine.commandHasTagForm(anyString())).thenReturn(true); + server.getPluginManager().registerEvents(listener, super.plugin); + var player = createPlayer(); + + var result = + dev.cyr1en.promptpaper.execution.dispatch.CommandEventBridge.fire( + player, "warn Steve reason "); + + assertEquals("warn Steve reason ", result); + org.mockito.Mockito.verify(screenManager, org.mockito.Mockito.never()) + .startSession(any(), anyString()); + } + @Test void commandPrompterCommandIsNotIntercepted() { var player = createPlayer(); From 05edabe1d281a522c16bfb723b5b44191d0962f2 Mon Sep 17 00:00:00 2001 From: cyr1en Date: Sun, 8 Nov 2026 14:26:19 -0700 Subject: [PATCH 2/2] fix: tighten the finished prompt command event - Treat a listener cancel as cancelled: run ON_CANCEL actions and skip the error message, instead of reporting the command as completed. - Fire the event before granting attachment permissions, and run a rewritten command as the plain player. - Fire the event for the primary command only, not preset actions the player never typed. - Show the command that actually ran in the completed message. - Track the bridge's own event instead of a per-thread flag. --- .../coordinator/ExecutionCoordinator.java | 23 ++- .../dispatch/CommandEventBridge.java | 20 +- .../execution/dispatch/DispatchErrorKind.java | 4 +- .../execution/dispatch/DispatchOutcome.java | 19 +- .../PaperImmediateActionDispatcher.java | 3 +- .../PaperPrimaryCommandDispatcher.java | 123 ++++++------ .../listener/PlayerCommandListener.java | 3 +- .../ExecutionCoordinatorIntegrationTest.java | 92 +++++++++ .../PaperImmediateActionDispatcherTest.java | 30 +++ .../PaperPrimaryCommandDispatcherTest.java | 186 +++++++++++++++--- 10 files changed, 396 insertions(+), 107 deletions(-) diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinator.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinator.java index 7864f9c..612aebb 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinator.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinator.java @@ -1138,7 +1138,8 @@ private void handleDispatchOutcome( return; } - sendCompletedCommand(player, command); + sendCompletedCommand( + player, outcome.rewrittenCommand() != null ? outcome.rewrittenCommand() : command); runPostActions( player, @@ -1159,12 +1160,20 @@ private void handleDispatchOutcome( error != null && error.detail() != null ? error.detail() : "post-action failed"); }); } else { - String detail = - outcome != null && outcome.error() != null ? outcome.error().detail() : "dispatch failed"; - reportCommandFailure(player, executionId, initiatorUuid, detail); + // A listener cancelling the command is not an error, so the player gets no error message. + boolean cancelledByListener = outcome != null && outcome.isCancelledByListener(); + ExecutionStage terminalStage = + cancelledByListener ? ExecutionStage.CANCELLED : ExecutionStage.ERROR; + if (!cancelledByListener) { + String detail = + outcome != null && outcome.error() != null + ? outcome.error().detail() + : "dispatch failed"; + reportCommandFailure(player, executionId, initiatorUuid, detail); + } if (!instance.tryTransitionTo(ExecutionStage.POST_ACTIONS)) { - instance.tryTransitionTo(ExecutionStage.ERROR); + instance.tryTransitionTo(terminalStage); registry.removeIfExact(initiatorUuid, executionId); return; } @@ -1175,11 +1184,11 @@ private void handleDispatchOutcome( initiatorExecutor, ExecutionPolicy.ON_CANCEL, () -> { - instance.tryTransitionTo(ExecutionStage.ERROR); + instance.tryTransitionTo(terminalStage); registry.removeIfExact(initiatorUuid, executionId); }, error -> { - instance.tryTransitionTo(ExecutionStage.ERROR); + instance.tryTransitionTo(terminalStage); registry.removeIfExact(initiatorUuid, executionId); }); } diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java index 5349d02..455a311 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/CommandEventBridge.java @@ -11,13 +11,16 @@ */ public final class CommandEventBridge { - private static final ThreadLocal FIRING = ThreadLocal.withInitial(() -> false); + private static final ThreadLocal CURRENT = new ThreadLocal<>(); private CommandEventBridge() {} - /** True while the bridge fires its own event, so the prompt listener can skip it. */ - public static boolean isFiring() { - return FIRING.get(); + /** + * True only for the event the bridge is firing, so the prompt listener can skip it. Any other + * command event raised by a listener during that call is still treated normally. + */ + public static boolean isBridgeEvent(PlayerCommandPreprocessEvent event) { + return event != null && CURRENT.get() == event; } /** @@ -26,11 +29,16 @@ public static boolean isFiring() { */ public static String fire(Player player, String command) { PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(player, "/" + command); - FIRING.set(true); + PlayerCommandPreprocessEvent previous = CURRENT.get(); + CURRENT.set(event); try { Bukkit.getPluginManager().callEvent(event); } finally { - FIRING.set(false); + if (previous == null) { + CURRENT.remove(); + } else { + CURRENT.set(previous); + } } if (event.isCancelled()) { return null; diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchErrorKind.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchErrorKind.java index 517fd78..d279738 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchErrorKind.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchErrorKind.java @@ -15,5 +15,7 @@ public enum DispatchErrorKind { /** Bukkit dispatch threw an unhandled exception. */ EXCEPTION_THROWN, /** Target or initiator player entity scheduler was retired or unavailable. */ - SCHEDULER_RETIRED + SCHEDULER_RETIRED, + /** A command event listener cancelled the finished command, so it never ran. */ + CANCELLED_BY_LISTENER } diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchOutcome.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchOutcome.java index b7fe2bf..2786187 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchOutcome.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/DispatchOutcome.java @@ -4,7 +4,7 @@ import java.util.Optional; /** Immutable result of a command dispatch invocation. */ -public record DispatchOutcome(boolean isSuccess, DispatchError error) { +public record DispatchOutcome(boolean isSuccess, DispatchError error, String rewrittenCommand) { public DispatchOutcome { if (!isSuccess && error == null) { error = DispatchError.of(DispatchErrorKind.DISPATCH_RETURNED_FALSE, "dispatch failed"); @@ -12,12 +12,21 @@ public record DispatchOutcome(boolean isSuccess, DispatchError error) { } public static DispatchOutcome success() { - return new DispatchOutcome(true, null); + return new DispatchOutcome(true, null, null); + } + + /** Success for a command that a listener replaced; carries the command that actually ran. */ + public static DispatchOutcome success(String rewrittenCommand) { + return new DispatchOutcome(true, null, rewrittenCommand); + } + + public static DispatchOutcome cancelledByListener() { + return failure(DispatchErrorKind.CANCELLED_BY_LISTENER, "cancelled by a command listener"); } public static DispatchOutcome failure(DispatchError error) { Objects.requireNonNull(error, "error must not be null for failure outcome"); - return new DispatchOutcome(false, error); + return new DispatchOutcome(false, error, null); } public static DispatchOutcome failure(DispatchErrorKind kind, String detail) { @@ -32,6 +41,10 @@ public boolean isFailure() { return !isSuccess; } + public boolean isCancelledByListener() { + return error != null && error.kind() == DispatchErrorKind.CANCELLED_BY_LISTENER; + } + public Optional optionalError() { return Optional.ofNullable(error); } diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java index 750622f..08441ce 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcher.java @@ -92,8 +92,7 @@ private void dispatchPlayer( initiator.execute( () -> { try { - String finalCommand = CommandEventBridge.fire(player, command); - boolean ok = finalCommand == null || Bukkit.dispatchCommand(player, finalCommand); + boolean ok = Bukkit.dispatchCommand(player, command); if (ok) { callback.onComplete(DispatchOutcome.success()); } else { diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java index 0dbfff7..9e40c07 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcher.java @@ -4,6 +4,7 @@ import dev.cyr1en.promptpaper.util.Scheduler; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; import org.bukkit.Bukkit; import org.bukkit.entity.Player; import org.bukkit.permissions.PermissionAttachment; @@ -79,19 +80,13 @@ private void dispatchPlayer( PlayerExecutor initiator, Player player, String command, PrimaryDispatchCallback callback) { initiator.execute( () -> { + DispatchOutcome outcome; try { - String finalCommand = CommandEventBridge.fire(player, command); - boolean ok = finalCommand == null || Bukkit.dispatchCommand(player, finalCommand); - if (ok) { - callback.onComplete(DispatchOutcome.success()); - } else { - callback.onComplete( - DispatchOutcome.failure( - DispatchErrorKind.DISPATCH_RETURNED_FALSE, "dispatch returned false")); - } + outcome = fireAndDispatch(player, command, () -> runAsPlayer(player, command)); } catch (Throwable t) { - callback.onComplete(DispatchOutcome.failure(DispatchErrorKind.EXCEPTION_THROWN, t)); + outcome = DispatchOutcome.failure(DispatchErrorKind.EXCEPTION_THROWN, t); } + callback.onComplete(outcome); }, () -> callback.onComplete( @@ -99,6 +94,32 @@ private void dispatchPlayer( DispatchErrorKind.SCHEDULER_RETIRED, "player scheduler retired"))); } + /** + * Fires the command event, then runs the command. An unchanged command goes to {@code + * runUnchanged}; a command a listener replaced always runs as the plain player, because any + * temporary permissions were granted for the original command only. + */ + private DispatchOutcome fireAndDispatch( + Player player, String command, Supplier runUnchanged) { + String finalCommand = CommandEventBridge.fire(player, command); + if (finalCommand == null) { + return DispatchOutcome.cancelledByListener(); + } + if (finalCommand.equals(command)) { + return runUnchanged.get(); + } + DispatchOutcome outcome = runAsPlayer(player, finalCommand); + return outcome.isSuccess() ? DispatchOutcome.success(finalCommand) : outcome; + } + + private DispatchOutcome runAsPlayer(Player player, String command) { + if (Bukkit.dispatchCommand(player, command)) { + return DispatchOutcome.success(); + } + return DispatchOutcome.failure( + DispatchErrorKind.DISPATCH_RETURNED_FALSE, "dispatch returned false"); + } + private void dispatchConsole(String command, PrimaryDispatchCallback callback) { try { scheduler.runSync( @@ -147,58 +168,14 @@ private void dispatchAttachment( initiator.execute( () -> { - PermissionAttachment attachment; - try { - attachment = player.addAttachment(plugin); - } catch (Throwable t) { - callback.onComplete( - DispatchOutcome.failure(DispatchErrorKind.ATTACHMENT_CREATION_FAILED, t)); - return; - } - - if (attachment == null) { - callback.onComplete( - DispatchOutcome.failure( - DispatchErrorKind.ATTACHMENT_CREATION_FAILED, - "unable to create permission attachment")); - return; - } - - final PermissionAttachment finalAttachment = attachment; - AtomicBoolean removed = new AtomicBoolean(false); - Runnable removeTask = - () -> { - if (removed.compareAndSet(false, true)) { - try { - player.removeAttachment(finalAttachment); - } catch (Throwable ignored) { - } - } - }; - DispatchOutcome outcome; try { - for (String perm : context.permissionSnapshot()) { - attachment.setPermission(perm, true); - } - attachment.getPermissible().recalculatePermissions(); - - String finalCommand = CommandEventBridge.fire(player, command); - boolean ok = finalCommand == null || Bukkit.dispatchCommand(player, finalCommand); - if (ok) { - outcome = DispatchOutcome.success(); - } else { - outcome = - DispatchOutcome.failure( - DispatchErrorKind.DISPATCH_RETURNED_FALSE, "dispatch returned false"); - } + // The event fires before the grant so listeners see the player's real permissions. + outcome = + fireAndDispatch(player, command, () -> runWithAttachment(player, command, context)); } catch (Throwable t) { outcome = DispatchOutcome.failure(DispatchErrorKind.EXCEPTION_THROWN, t); } - - // The attachment exists only for the synchronous dispatch call. Completion is - // never published while elevated permissions remain attached. - removeTask.run(); callback.onComplete(outcome); }, () -> @@ -206,4 +183,36 @@ private void dispatchAttachment( DispatchOutcome.failure( DispatchErrorKind.SCHEDULER_RETIRED, "player scheduler retired"))); } + + private DispatchOutcome runWithAttachment( + Player player, String command, PermissionAttachmentContext context) { + PermissionAttachment attachment; + try { + attachment = player.addAttachment(plugin); + } catch (Throwable t) { + return DispatchOutcome.failure(DispatchErrorKind.ATTACHMENT_CREATION_FAILED, t); + } + + if (attachment == null) { + return DispatchOutcome.failure( + DispatchErrorKind.ATTACHMENT_CREATION_FAILED, "unable to create permission attachment"); + } + + // The attachment exists only for the synchronous dispatch call. Completion is + // never published while elevated permissions remain attached. + try { + for (String perm : context.permissionSnapshot()) { + attachment.setPermission(perm, true); + } + attachment.getPermissible().recalculatePermissions(); + return runAsPlayer(player, command); + } catch (Throwable t) { + return DispatchOutcome.failure(DispatchErrorKind.EXCEPTION_THROWN, t); + } finally { + try { + player.removeAttachment(attachment); + } catch (Throwable ignored) { + } + } + } } diff --git a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java index 4133073..f012cd3 100644 --- a/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java +++ b/prompt-paper/src/main/java/dev/cyr1en/promptpaper/listener/PlayerCommandListener.java @@ -3,6 +3,7 @@ import dev.cyr1en.promptpaper.CommandPrompter; import dev.cyr1en.promptpaper.engine.InterceptResult; import dev.cyr1en.promptpaper.engine.PromptEngine; +import dev.cyr1en.promptpaper.execution.dispatch.CommandEventBridge; import dev.cyr1en.promptpaper.screen.ScreenManager; import java.util.Locale; import org.bukkit.event.EventHandler; @@ -52,7 +53,7 @@ public void onPlayerCommand(PlayerCommandPreprocessEvent event) { plugin.getPluginLogger().debug("Command already cancelled, skipping"); return; } - if (dev.cyr1en.promptpaper.execution.dispatch.CommandEventBridge.isFiring()) { + if (CommandEventBridge.isBridgeEvent(event)) { // Finished prompt command, re-fired for other plugins. Never prompt it again. return; } diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinatorIntegrationTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinatorIntegrationTest.java index d11be90..f28343b 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinatorIntegrationTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/coordinator/ExecutionCoordinatorIntegrationTest.java @@ -30,6 +30,7 @@ import dev.cyr1en.promptpaper.execution.dispatch.PrimaryDispatchCallback; import dev.cyr1en.promptpaper.execution.postaction.PostActionScheduler; import dev.cyr1en.promptpaper.execution.runtime.DispatchContextSnapshot; +import dev.cyr1en.promptpaper.execution.runtime.ExecutionPlanInstance; import dev.cyr1en.promptpaper.execution.runtime.ExecutionRegistry; import dev.cyr1en.promptpaper.execution.runtime.ExecutionStage; import dev.cyr1en.promptpaper.execution.runtime.InputCompletion; @@ -50,6 +51,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.mockbukkit.mockbukkit.entity.PlayerMock; @DisplayName("Execution Coordinator and Runtime Integration Tests (Phase 5.3)") class ExecutionCoordinatorIntegrationTest extends MockBukkitTest { @@ -1956,4 +1958,94 @@ void loggingRegressionSanitizesSecretsAndMarkup() { deserialized.clickEvent(), "Deserialized component must not have click event from injected markup"); } + + private ExecutionPlanInstance coordinateWithOutcome( + PlayerMock player, List log, DispatchOutcome outcome) { + registerMockCommand("successpcm", true, log); + registerMockCommand("cancelpcm", true, log); + + var capturedCallback = new AtomicReference(); + PrimaryCommandDispatcher controllableDispatcher = + (request, callback) -> capturedCallback.set(callback); + var customCoordinator = + new ExecutionCoordinator( + plugin, + engine, + registry, + controllableDispatcher, + immediateActionDispatcher, + p -> (task, retired) -> task.run()); + + var successPcm = + new PostCommandMeta("successpcm", new int[0], 0, false, DispatchTarget.PASSTHROUGH, false); + var cancelPcm = + new PostCommandMeta("cancelpcm", new int[0], 0, true, DispatchTarget.PASSTHROUGH, false); + var sessionResult = + new SessionResult("original y", List.of(), List.of(successPcm), List.of(cancelPcm)); + var parsed = + new ParsedCommand( + "original y", List.of(), List.of(successPcm, cancelPcm), ParserConfig.ANGLE_BRACKETS); + var completion = + InputCompletion.of( + player.getUniqueId(), + 1L, + 1L, + sessionResult, + ExecutionPlanAdapter.fromParsedCommand(parsed), + PresetSnapshot.empty(), + DispatchContextSnapshot.player()); + + var instance = customCoordinator.coordinate(player, completion, sessionResult).orElseThrow(); + capturedCallback.get().onComplete(outcome); + return instance; + } + + @Test + @DisplayName("Listener-cancelled primary: runs ON_CANCEL actions with no error message") + void listenerCancelledPrimaryRunsCancelActionsSilently() { + var player = createPlayer("TestListenerCancel"); + var log = new ArrayList(); + + var instance = coordinateWithOutcome(player, log, DispatchOutcome.cancelledByListener()); + + assertEquals(ExecutionStage.CANCELLED, instance.getStage()); + assertEquals(List.of("cancelpcm"), log); + org.mockito.Mockito.verify(i18n, org.mockito.Mockito.never()) + .get( + org.mockito.ArgumentMatchers.eq("prompt.error.command_failed"), + org.mockito.ArgumentMatchers.any(), + org.mockito.ArgumentMatchers.any(Placeholder[].class)); + assertFalse(registry.hasActiveExecution(player.getUniqueId())); + } + + @Test + @DisplayName("Failed primary: still reports the error message") + void failedPrimaryStillReportsError() { + var player = createPlayer("TestFailureReported"); + var log = new ArrayList(); + + var instance = + coordinateWithOutcome( + player, + log, + DispatchOutcome.failure(DispatchErrorKind.DISPATCH_RETURNED_FALSE, "nope")); + + assertEquals(ExecutionStage.ERROR, instance.getStage()); + org.mockito.Mockito.verify(i18n) + .get( + org.mockito.ArgumentMatchers.eq("prompt.error.command_failed"), + org.mockito.ArgumentMatchers.any(), + org.mockito.ArgumentMatchers.any(Placeholder[].class)); + } + + @Test + @DisplayName("Rewritten primary: the completed message shows the command that ran") + void rewrittenPrimaryShowsCommandThatRan() { + var player = createPlayer("TestRewriteMessage"); + org.mockito.Mockito.when(config.showCompleted()).thenReturn(true); + + coordinateWithOutcome(player, new ArrayList<>(), DispatchOutcome.success("rewritten x")); + + assertEquals("rewritten x", player.nextMessage()); + } } diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcherTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcherTest.java index 65b4f5e..a29224a 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcherTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperImmediateActionDispatcherTest.java @@ -9,6 +9,10 @@ import java.util.concurrent.atomic.AtomicReference; import org.bukkit.command.Command; import org.bukkit.command.CommandSender; +import org.bukkit.event.EventHandler; +import org.bukkit.event.HandlerList; +import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerCommandPreprocessEvent; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -70,6 +74,32 @@ void dispatchesPlayerActionSuccessfully() { assertTrue(outcomeRef.get().isSuccess()); } + @Test + @DisplayName("PLAYER action: fires no command event because the player did not type it") + void playerActionFiresNoCommandEvent() { + var player = createPlayer("PlayerActionNoEvent"); + registerMockCommand("actnoevent", true); + var eventCount = new AtomicInteger(0); + Listener listener = + new Listener() { + @EventHandler + public void on(PlayerCommandPreprocessEvent e) { + eventCount.incrementAndGet(); + } + }; + server.getPluginManager().registerEvents(listener, plugin); + + var outcomeRef = new AtomicReference(); + try { + dispatcher.dispatch(ImmediateActionRequest.player(player, "/actnoevent"), outcomeRef::set); + } finally { + HandlerList.unregisterAll(listener); + } + + assertTrue(outcomeRef.get().isSuccess()); + assertEquals(0, eventCount.get()); + } + @Test @DisplayName("CONSOLE action: allows trusted preset when player holds promptpaper.pcm.console") void allowsTrustedPresetForAuthorizedPlayer() { diff --git a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java index c58fca3..fb9eefe 100644 --- a/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java +++ b/prompt-paper/src/test/java/dev/cyr1en/promptpaper/execution/dispatch/PaperPrimaryCommandDispatcherTest.java @@ -5,12 +5,19 @@ import dev.cyr1en.promptpaper.MockBukkitTest; import dev.cyr1en.promptpaper.custom.PlayerExecutor; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import org.bukkit.command.Command; import org.bukkit.command.CommandSender; +import org.bukkit.event.EventHandler; +import org.bukkit.event.HandlerList; +import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerCommandPreprocessEvent; import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -20,11 +27,19 @@ class PaperPrimaryCommandDispatcherTest extends MockBukkitTest { private PaperPrimaryCommandDispatcher dispatcher; + /** Handler lists are static and outlive MockBukkit, so each test must remove its listeners. */ + private final List listeners = new ArrayList<>(); + @BeforeEach void setupDispatcher() { dispatcher = new PaperPrimaryCommandDispatcher(plugin, scheduler); } + @AfterEach + void unregisterListeners() { + listeners.forEach(HandlerList::unregisterAll); + } + private void registerMockCommand(String name, boolean succeed) { Command cmd = new Command(name) { @@ -63,24 +78,46 @@ public boolean execute( return cmd; } + private void registerPermissionRecordingCommand( + String name, String permission, List results) { + Command cmd = + new Command(name) { + @Override + public boolean execute( + @NotNull CommandSender sender, @NotNull String commandLabel, @NotNull String[] args) { + results.add(sender.hasPermission(permission)); + return true; + } + }; + server.getCommandMap().register(name, cmd); + } + + private void onCommandEvent(Consumer handler) { + Listener listener = + new Listener() { + @EventHandler + public void on(PlayerCommandPreprocessEvent e) { + handler.accept(e); + } + }; + listeners.add(listener); + server.getPluginManager().registerEvents(listener, plugin); + } + @Test @DisplayName("PLAYER mode: fires PlayerCommandPreprocessEvent with the finished command") void firesPreprocessEventBeforeDispatch() { var player = createPlayer("EventPlayer"); - var calls = new java.util.ArrayList(); + var calls = new ArrayList(); recordingCommand("eventcmd", calls); - var seen = new java.util.ArrayList(); - server - .getPluginManager() - .registerEvents( - new org.bukkit.event.Listener() { - @org.bukkit.event.EventHandler - public void on(org.bukkit.event.player.PlayerCommandPreprocessEvent e) { - seen.add(e.getMessage()); - assertTrue(CommandEventBridge.isFiring()); - } - }, - plugin); + var seen = new ArrayList(); + var fired = new AtomicReference(); + onCommandEvent( + e -> { + seen.add(e.getMessage()); + fired.set(e); + assertTrue(CommandEventBridge.isBridgeEvent(e)); + }); var outcomeRef = new AtomicReference(); dispatcher.dispatch(PrimaryDispatchRequest.player(player, "/eventcmd a b"), outcomeRef::set); @@ -88,30 +125,24 @@ public void on(org.bukkit.event.player.PlayerCommandPreprocessEvent e) { assertEquals(List.of("/eventcmd a b"), seen); assertEquals(List.of("eventcmd a b"), calls); assertTrue(outcomeRef.get().isSuccess()); - assertFalse(CommandEventBridge.isFiring()); + assertFalse(CommandEventBridge.isBridgeEvent(fired.get())); } @Test @DisplayName("PLAYER mode: a listener can rewrite or cancel the finished command") void preprocessListenerCanRewriteOrCancel() { var player = createPlayer("RewritePlayer"); - var calls = new java.util.ArrayList(); + var calls = new ArrayList(); recordingCommand("rewritten", calls); recordingCommand("blockedcmd", calls); - server - .getPluginManager() - .registerEvents( - new org.bukkit.event.Listener() { - @org.bukkit.event.EventHandler - public void on(org.bukkit.event.player.PlayerCommandPreprocessEvent e) { - if (e.getMessage().startsWith("/original")) { - e.setMessage("/rewritten x"); - } else if (e.getMessage().startsWith("/blockedcmd")) { - e.setCancelled(true); - } - } - }, - plugin); + onCommandEvent( + e -> { + if (e.getMessage().startsWith("/original")) { + e.setMessage("/rewritten x"); + } else if (e.getMessage().startsWith("/blockedcmd")) { + e.setCancelled(true); + } + }); var first = new AtomicReference(); dispatcher.dispatch(PrimaryDispatchRequest.player(player, "/original y"), first::set); @@ -120,7 +151,102 @@ public void on(org.bukkit.event.player.PlayerCommandPreprocessEvent e) { assertEquals(List.of("rewritten x"), calls); assertTrue(first.get().isSuccess()); - assertTrue(second.get().isSuccess(), "a cancelled command is handled, like a typed one"); + assertEquals("rewritten x", first.get().rewrittenCommand()); + assertTrue(second.get().isFailure(), "a cancelled command never ran, so it is not a success"); + assertTrue(second.get().isCancelledByListener()); + } + + @Test + @DisplayName("PLAYER mode: an unchanged command reports no rewrite") + void unchangedCommandReportsNoRewrite() { + var player = createPlayer("NoRewritePlayer"); + registerMockCommand("plaincmd", true); + + var outcomeRef = new AtomicReference(); + dispatcher.dispatch(PrimaryDispatchRequest.player(player, "/plaincmd a"), outcomeRef::set); + + assertTrue(outcomeRef.get().isSuccess()); + assertNull(outcomeRef.get().rewrittenCommand()); + } + + @Test + @DisplayName("ATTACHMENT mode: listeners see real permissions, the command runs elevated") + void attachmentEventFiresBeforePermissionGrant() { + var player = createPlayer("AttachEventPlayer"); + var permKey = "test.event.perm"; + var permDuringEvent = new AtomicReference(); + var permDuringCommand = new ArrayList(); + registerPermissionRecordingCommand("attachevent", permKey, permDuringCommand); + onCommandEvent(e -> permDuringEvent.set(e.getPlayer().hasPermission(permKey))); + + var context = PermissionAttachmentContext.of("KEY", List.of(permKey)); + var outcomeRef = new AtomicReference(); + dispatcher.dispatch( + PrimaryDispatchRequest.attachment(player, "/attachevent", context), outcomeRef::set); + + assertEquals(Boolean.FALSE, permDuringEvent.get()); + assertEquals(List.of(true), permDuringCommand); + assertTrue(outcomeRef.get().isSuccess()); + assertFalse(player.hasPermission(permKey)); + } + + @Test + @DisplayName("ATTACHMENT mode: a rewritten command runs without the temporary permissions") + void attachmentRewrittenCommandIsNotElevated() { + var player = createPlayer("AttachRewritePlayer"); + var permKey = "test.rewrite.perm"; + var permDuringCommand = new ArrayList(); + registerPermissionRecordingCommand("attachother", permKey, permDuringCommand); + registerMockCommand("attachoriginal", true); + onCommandEvent(e -> e.setMessage("/attachother")); + + var context = PermissionAttachmentContext.of("KEY", List.of(permKey)); + var outcomeRef = new AtomicReference(); + dispatcher.dispatch( + PrimaryDispatchRequest.attachment(player, "/attachoriginal", context), outcomeRef::set); + + assertEquals(List.of(false), permDuringCommand); + assertTrue(outcomeRef.get().isSuccess()); + assertEquals("attachother", outcomeRef.get().rewrittenCommand()); + } + + @Test + @DisplayName("ATTACHMENT mode: a cancelled command never runs") + void attachmentCancelledCommandNeverRuns() { + var player = createPlayer("AttachCancelPlayer"); + var calls = new ArrayList(); + recordingCommand("attachblocked", calls); + onCommandEvent(e -> e.setCancelled(true)); + + var context = PermissionAttachmentContext.of("KEY", List.of("test.cancel.perm")); + var outcomeRef = new AtomicReference(); + dispatcher.dispatch( + PrimaryDispatchRequest.attachment(player, "/attachblocked", context), outcomeRef::set); + + assertTrue(calls.isEmpty()); + assertTrue(outcomeRef.get().isCancelledByListener()); + } + + @Test + @DisplayName("Bridge: only its own event is skipped, and nested fires restore the outer event") + void bridgeTracksOnlyItsOwnEvent() { + var player = createPlayer("NestedPlayer"); + var unrelatedSkipped = new AtomicReference(); + var outerStillTracked = new AtomicReference(); + onCommandEvent( + e -> { + if (e.getMessage().equals("/outer")) { + unrelatedSkipped.set( + CommandEventBridge.isBridgeEvent(new PlayerCommandPreprocessEvent(player, "/x"))); + CommandEventBridge.fire(player, "inner"); + outerStillTracked.set(CommandEventBridge.isBridgeEvent(e)); + } + }); + + CommandEventBridge.fire(player, "outer"); + + assertEquals(Boolean.FALSE, unrelatedSkipped.get()); + assertEquals(Boolean.TRUE, outerStillTracked.get()); } @Test