Skip to content

Declare command metadata and permission in plugin.yml, and stop /fly falling through to usage - #11

Merged
dmccoystephenson merged 1 commit into
mainfrom
feature/declare-command-metadata-and-fix-return
Aug 27, 2026
Merged

Declare command metadata and permission in plugin.yml, and stop /fly falling through to usage#11
dmccoystephenson merged 1 commit into
mainfrom
feature/declare-command-metadata-and-fix-return

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

  • A permissions: block is added to plugin.yml, declaring FlyCommand.fly with default: op and the description already published in USER_GUIDE.md. The node is checked by Main.java and documented in two files, but was declared nowhere.
  • description: and usage: are added under the Fly command key, with the description taken verbatim from COMMANDS.md.
  • return true; is added to the permission-denied branch of onCommand, so the plugin's own alert is the only message shown to a player lacking the permission.
  • @Override is added to onCommand, matching onEnable and onDisable in the same class.
  • The user-visible portions of the above are recorded under ## [Unreleased] in CHANGELOG.md.

Why these three issues are batched

The manifest change and the return-value change are coupled and should not be separated. Before this branch, the permission-denied branch fell through to the trailing return false, but PluginCommand.execute only sends a usage message when the command's usage string is non-empty — and no usage: was declared, so nothing was in fact printed after the alert. Declaring a usage: for issue #5 is precisely what would have made issue #6's defect start being observed by players. Landing #5 alone would therefore have introduced the double message that #6 describes.

This refines what issue #6 states about the current observable behaviour: the second line it describes is latent rather than already visible. The defect in the return value is real either way, and the fix is unchanged.

Validation

Mechanical checks (run on the branch head; output quoted)

Manifest and source agreement:

plugin.yml:1:main: me.Daniel.FlyCommand.Main
Main.java:1:package me.Daniel.FlyCommand;
Main.java:8:public class Main extends JavaPlugin {

main: equals <package>.<class> and matches the file's path under me/.

Every permission checked by the source is declared and documented:

Main.java:29:  if (player.hasPermission("FlyCommand.fly")) {
plugin.yml:12:  FlyCommand.fly:
USER_GUIDE.md:21:| `FlyCommand.fly` | `op` | Allows the player to toggle flight. |
COMMANDS.md:5:| `/fly` | `FlyCommand.fly` | Toggles flight mode on or off for the executing player. |

The declared default: op matches the op published in the USER_GUIDE.md table.

Every handled label has a manifest key:

Main.java:22:  if (label.equalsIgnoreCase("fly")) {
plugin.yml:6:commands:
plugin.yml:7:  Fly:

The manifest parses, and parses to the intended structure:

plugin.yml parses
{"main": "me.Daniel.FlyCommand.Main", "name": "FlyCommand", "version": 1.0,
 "api-version": 1.13,
 "commands": {"Fly": {"description": "Toggles flight mode on or off for the executing player.",
                      "usage": "/<command>"}},
 "permissions": {"FlyCommand.fly": {"description": "Allows the player to toggle flight.",
                                    "default": "op"}}}

Regression evidence for the manifest defect

The manifest was reverted to origin/main and the permission check was re-run. It reported the defect — plugin.yml produced no match while both documentation files continued to name the node:

USER_GUIDE.md:21:| `FlyCommand.fly` | `op` | Allows the player to toggle flight. |
COMMANDS.md:5:| `/fly` | `FlyCommand.fly` | Toggles flight mode on or off for the executing player. |

The manifest change was then restored and the check came back clean. The assertion is therefore specific enough to observe the defect, not a false negative.

What is NOT verified here

The two Main.java changes are behavioural and compile-time respectively, and nothing in this repository can compile or run the plugin. There is no build system, and both GitHub Actions workflows contain a single actions/checkout@v4 step with no run: steps — a green check confirms only that the repository could be cloned. That gap is tracked in #9.

Both source changes are therefore recorded UNVERIFIED and require a human with a server before release.

On-server reproduction recipe

A Spigot or Paper server at API 1.13 or later is required.

  1. Build the JAR by hand, as documented under Building in README.md, and place it in plugins/. Start the server.
  2. Confirm the plugin loads and that /fly is registered. /help fly should now show the declared description rather than nothing.
  3. Permission denial (issue Permission-denied branch falls through to return false, printing usage after the alert #6). Join with an account that is neither an operator nor a holder of FlyCommand.fly, and run /fly.
    • Expected on this branch: exactly one line, Alert: Permission 'FlyCommand.fly' required.
    • Expected if return true; is removed while the new usage: is kept: that alert followed by a second line, /fly.
  4. Permission grant. Grant FlyCommand.fly to that same non-operator account and run /fly twice, confirming flight is enabled and then disabled.
  5. Operator default (issue plugin.yml declares no permissions block and no command metadata #5). Join as an operator holding no explicit grant and run /fly, confirming toggling still works — default: op is intended to codify the pre-existing fallback rather than change it, and that is the assumption most worth checking on a real server.
  6. Console. Run /fly from the server console, confirming Alert: Can't be used by console. is shown and no error is logged.
  7. Annotation (issue onCommand is missing the @Override annotation #7). A successful compile against the Spigot API JAR is itself the evidence that the signature genuinely overrides CommandExecutor.onCommand.

Issues deferred this cycle

  • Flight-toggle message prints a raw boolean #8 — flight-toggle message prints a raw boolean. Deferred. The wording of user-facing output is a separate concern from the command's declaration contract, and the change would need its own USER_GUIDE.md and CHANGELOG.md treatment. It is a good candidate for the next cycle.
  • No automated verification exists: no build system, and CI is checkout-only #9 — no automated verification exists. Deferred as charter-level. Adopting a build system changes the declared stack, is coupled to plugin.yml's main: and to the project-structure entry in .github/copilot-instructions.md, and maintainer authorization is requested on that issue before any of it is started.

Closes #5
Closes #6
Closes #7


This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

…h to usage

Adds a permissions block to plugin.yml declaring FlyCommand.fly with default
op, plus description and usage metadata for the Fly command, so the manifest
matches what USER_GUIDE.md, COMMANDS.md and Main.java already claim.

Returning true from the permission-denied branch of onCommand is part of the
same change rather than a separate one: with a usage string now declared,
falling through to the trailing return false would newly print that usage line
underneath the plugin's own permission alert.

Adds @OverRide to onCommand, matching onEnable and onDisable.

Closes #5
Closes #6
Closes #7

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review

Posted as a plain comment rather than a formal review: this is the authoring session's own rubric pass, not independent review.

External anchor

The mechanical consistency checks are the anchor for this repository — there is no build system, and both workflows are checkout-only. They were run on the branch head and are green; their output is quoted in the PR description. gh pr checks 11 reports build pending, which carries no signal in either direction and is not scored below.

Rubric

  • Scope: PASS — three files are touched (plugin.yml, me/Daniel/FlyCommand/Main.java, CHANGELOG.md), 16 insertions and 1 deletion. Each hunk maps to one of plugin.yml declares no permissions block and no command metadata #5, Permission-denied branch falls through to return false, printing usage after the alert #6, onCommand is missing the @Override annotation #7. No reformatting: the CRLF line endings and tab indentation of both non-Markdown files are preserved, which is why the diff shows two added lines rather than a whole-file rewrite.
  • Validation-new: PASS — the manifest change is exercised by the permission and command-key checks quoted in the PR description. The two source changes are covered by numbered on-server steps naming the server version, the exact command, the permission held by the test account, and the expected before/after messages.
  • Validation-fix: PARTIAL
  • Sibling structure: PASS — no new file is created.
  • Sibling renames: PASS — no identifier is renamed.
  • Docs: PASS — every row of the documentation sources-of-truth table was re-checked. USER_GUIDE.md's op default now matches a declared default: op; COMMANDS.md's description is reproduced verbatim in the manifest; CONFIG.md's claim that "all behaviour is controlled via the plugin.yml permissions" is made true by this change rather than contradicted by it; README.md's manual-validation step 5 remains accurate. .github/copilot-instructions.md was treated as read-only and needs no change — the stack, build tool, API version and project-structure entry all remain true.
  • Issue resolution: PASS — each issue's named surface area is changed: a permissions: block plus command metadata for plugin.yml declares no permissions block and no command metadata #5, return true; in the permission-denied branch for Permission-denied branch falls through to return false, printing usage after the alert #6, @Override on onCommand for onCommand is missing the @Override annotation #7. None is partially resolved.
  • Manual validation: PASS — quoted in the PR description.
  • Manifest declaration: PASSFlyCommand.fly is the only permission string in the source (Main.java:29) and in the docs, and it is declared at plugin.yml:12 with default: op, matching USER_GUIDE.md:21.
  • Command declaration: PASSlabel.equalsIgnoreCase("fly") at Main.java:22 is matched by the Fly key at plugin.yml:7; Bukkit resolves command names case-insensitively.
  • Main-class resolution: PASSmain: me.Daniel.FlyCommand.Main equals package me.Daniel.FlyCommand plus public class Main, and matches me/Daniel/FlyCommand/Main.java on disk.
  • Override annotations: PASSgrep -c '@Override' returns 3, one for each of onEnable, onDisable and onCommand.
  • No stack change: PASS — no pom.xml, no build.gradle, no src/main/java. grep -n 'import ' shows only four org.bukkit.* imports.
  • Changelog: PASS — two ### Added bullets and two ### Fixed bullets under ## [Unreleased], in Keep a Changelog form.
  • No harness leakage: PASSgit diff --name-only origin/main...HEAD lists exactly CHANGELOG.md, me/Daniel/FlyCommand/Main.java, plugin.yml. This repository has no .gitignore, so staging was done by filename.

Findings folded in from the diff (no inline anchors used)

  • plugin.yml:9 — usage: /<command> is now unreachable as a fallback message, by design. After this change, every path through onCommand that matches the label returns true, so the trailing return false is only reachable for a label the executor is never registered for. The declared usage: therefore serves /help fly output rather than a failure fallback. That is the intended outcome and not a defect, but it is worth stating plainly so a later reader does not conclude the string is dead weight and delete it.
  • me/Daniel/FlyCommand/Main.java:38 — the trailing return false is now effectively dead. Removing it was deliberately not done: it is a harmless defensive default, and deleting it would widen this PR beyond the three issues it closes. A follow-up is not proposed.
  • The observable behaviour described in issue Permission-denied branch falls through to return false, printing usage after the alert #6 is corrected by this PR description. PluginCommand.execute sends a usage message only when the usage string is non-empty, so before this branch nothing was printed after the permission alert. The second line Permission-denied branch falls through to return false, printing usage after the alert #6 describes was latent, and would have been introduced by plugin.yml declares no permissions block and no command metadata #5's usage: declaration had Permission-denied branch falls through to return false, printing usage after the alert #6 not been fixed alongside it. The source was taken as authoritative over the issue text, and the two changes were batched for that reason.
  • plugin.yml:3 — version: 1.0 parses as a YAML float rather than a string. This is pre-existing and out of scope, and Bukkit stringifies the value when reading the manifest, so no change is proposed here.

Verdict

The manifest half of this change is verified. The two source changes are not, and cannot be by anything in this repository — see #9. Merge is therefore requested only after the on-server recipe in the PR description has been run by a maintainer with a Spigot or Paper server.


This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit c350882 into main Aug 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant