Run our own commands that Brigadier will not parse - #3042
Merged
Conversation
Brigadier hides nodes the sender may not use, and hiding is not passive: CommandNode#getRelevantNodes returns only the literal that matches the typed token, so the catch-all argument beside it is never considered, and CommandDispatcher#parseNodes then skips that literal when canUse is false. A player without the permission for a sub-command therefore got Brigadier's "Incorrect argument for command" instead of the command's own no-permission message, and the command never ran. Paper reports any parse failure as an unknown command, so pick our own command lines up there and dispatch them through the command tree, which checks its own permissions and reports for itself. Nothing has run at that point - Paper fails in its parse step, before execution - so there is no double execution. Two knock-on faults from the same report: - the matcher re-attaches tokens it could not match verbatim, so it suggested the very line that had just failed. Drop matches identical to the input. - accepting a suggestion ran it through Player#performCommand in a scheduled task with nothing catching what Bukkit throws, which is how the report's unhandled CommandException got out. Log it instead. Fixes BentoBoxWorld/Border#179 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E6rYr6Z66LQz9XTbnbUupy
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes BentoBoxWorld/Border#179 —
/oneblock border color greenproduced an unhandledCommandExceptionout of a BentoBox scheduled task, and never changed the border colour.Root cause
BrigadierCommandRegistrarputs a permission check in each sub-command literal's Brigadierrequirespredicate, so clients only see what they can use. Hiding a node that way is not passive:CommandNode#getRelevantNodesreturns only the literal matching the typed token, so the greedy catch-all argument beside it is never considered;CommandDispatcher#parseNodesthen silently skips that literal whencanUse(source)is false.So a player without
[gamemode].border.colorgets a parse that walksoneblock→borderand dead-ends atcolor: Brigadier'sINCORRECT_ARGUMENTat the cursor wherecolorstarts (exactly the...ck border <--[HERE]in the report). The command line never reachesCompositeCommand, so instead of "you do not have permission" the player gets a red Brigadier error, and nothing runs.Paper reports any
CommandSyntaxExceptionfromCommands#finishParsingas anUnknownCommandEvent, not just genuinely unknown commands. That produced the rest of the report:CommandMatcherre-attaches the tokens it could not match verbatim, so it suggested the very line that had just failed;Player#performCommandinside a scheduled task with nothing catching what Bukkit throws — the unhandledCommandExceptionin the report.Changes
DidYouMeanListener— a command line of ours that Brigadier refused to parse is dispatched through the command tree, which checks its own permissions and reports for itself; Brigadier's error is then dropped. Nothing has run at that point (Paper fails in its parse step, before execution), so there is no double execution.CommandsManager#resolveCommand— resolves a top level command from a typed first token: label, alias, or either inaddon:labelform.SuggestionsManager— never suggest a line identical to what was typed.DidYouMeanListener#acceptPending— log dispatch failures instead of letting them escape the scheduled task.Testing
New
UnparseableCommandTestreproduces the report end to end (permission-hidden sub-command → command reports for itself, nothing suggested back, nothing left pending, dispatch failures swallowed). Verified each test fails without the corresponding fix. Full suite green: 3391 tests, 0 failures.Not addressed here
BrigadierCommandRegistrar#refreshTreescompareschildCount(label)before and after the rebuild, butdispatcher.getRoot().getChild(label)returns Paper's wrapped (ApiMirrorRootNode) node while the merge lands on the NMS copy — so the count never changes andrefreshClients()is never called. #3039's symptom is likely still present on a real server; the unit tests pass only because they use a plainRootCommandNode. Worth a follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01E6rYr6Z66LQz9XTbnbUupy