# Plugin Review

> Read-only review of a Minecraft plugin's source against the common pitfalls — threading, memory leaks, events, commands, config, persistence, lifecycle, performance, and 2026 version drift. Use this when the user wants their plugin reviewed, audited, or checked for bugs/leaks/thread-safety before shipping, or asks "is my plugin correct / safe / production-ready?". Reports findings by severity; never edits. Reads the stack from .mcplugin/config.yml.

- Skill: `itamarb2010-jpg/plugin-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add itamarb2010-jpg/plugin-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/itamarb2010-jpg/plugin-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: itamarb2010-jpg (https://skillmd.com/u/itamarb2010-jpg)
- Updated: 2026-09-22
- Page: https://skillmd.com/skills/itamarb2010-jpg/plugin-review

---


# Review the plugin for pitfalls

A **read-only** audit of the plugin source against the toolkit's pitfalls list. Find real,
Minecraft-specific defects, cite each with `file:line`, and recommend a concrete fix. **Do not
edit anything** — this skill only reports.

## Phase 1: Load context

- Read `.mcplugin/config.yml` for `platform`, `mc_version`, `version_scheme`, `api_version`,
  `main_class`, `package`, `plugin_name`. Missing → note it and review generically (some checks,
  e.g. `main` matching or Folia-correctness, need the platform/version to be precise).
- Read `../../references/pitfalls.md` — the authoritative checklist (`Glob`
  `**/references/pitfalls.md` as fallback). Also skim `../../references/api/VERSION.md` for the
  version→api-version facts.
- Map the source: `Glob` `src/main/java/**/*.java`, read the main class, listeners, command
  classes; read `src/main/resources/plugin.yml` (or `paper-plugin.yml`). If the user named a
  focus path/class, prioritize it but still sanity-check the lifecycle glue.

## Phase 2: Systematic checks

Go category by category. `Grep` for the risky patterns, then read the surrounding code to confirm
(don't flag on a pattern match alone).

- **Threading** — Bukkit API touched off the main thread (world/entity/player/block/inventory
  work inside `runTaskAsynchronously`, `CompletableFuture`, raw `Thread`); blocking calls on the
  main thread (`Thread.sleep`, synchronous HTTP/JDBC/file I/O in handlers or `onEnable`). On
  **Folia**, flag `Bukkit.getScheduler()` assumptions — it needs region/entity/global schedulers.
- **Memory leaks** — `Player`/`Entity`/`Block` stored in long-lived fields/collections (should
  store `UUID` or a light location record and look up on demand); per-player state that's never
  cleared; tasks/listeners not cancelled/unregistered in `onDisable()`.
- **Events** — listener actually **registered** (`registerEvents`); handlers have `@EventHandler`
  and a single event param; `EventPriority` sensible and **no state mutation in `MONITOR`**;
  cancelled events handled (`isCancelled()` / `ignoreCancelled`); cancels use
  `setCancelled(true)`, not an early `return`.
- **Commands** — every `plugin.yml` command has an executor set in code; `getCommand("x")`
  null-guarded (null when not declared); `args.length` checked before indexing;
  `sender instanceof Player` checked before casting; permission nodes consistent between code and
  `plugin.yml`; tab completion present.
- **Config** — `saveDefaultConfig()` with a bundled `config.yml`; `reloadConfig()` before reads
  after on-disk edits and `saveConfig()` after programmatic changes; `getX(path)` results
  defaulted / null-checked.
- **Persistence** — prefer `PersistentDataContainer` over deprecated metadata/NBT hacks; one
  reused `NamespacedKey` per key (not recreated per call); DB/file I/O done **async**.
- **Lifecycle & correctness** — `main:` in the descriptor equals `<package>.<main_class>`;
  `api-version` set (per VERSION.md); cleanup in `onDisable()`; no heavy blocking work in
  `onEnable()`; null-checks on `ItemStack`/`ItemMeta`/`getClickedBlock()` and similar.
- **Performance** — heavy work in hot paths (`PlayerMoveEvent`, `BlockPhysicsEvent`, per-tick
  tasks); `PlayerMoveEvent` handlers that don't early-return when the block position is unchanged;
  scanning all players/entities every tick; uncached repeated lookups.
- **2026 version drift** — version parsing that assumes a leading `1.` (breaks on `26.1.x`);
  reliance on obfuscated NMS names on 26.x (mappings are native now); APIs asserted from memory
  for a target newer than VERSION.md documents — flag as "verify against live docs".

## Phase 3: Report — grouped by severity

Output findings in three tiers, each as `file:line — problem → concrete fix`:
- **Critical** — will crash, corrupt state, leak unboundedly, or stop the plugin loading
  (off-main-thread world access, `main` mismatch, unregistered core listener, main-thread
  blocking I/O).
- **Warning** — likely bug or real risk under load (missing cancel handling, `Player` held in a
  map, no `onDisable` cleanup, missing null guard).
- **Suggestion** — correctness/robustness/perf polish (add tab completion, reuse a
  `NamespacedKey`, early-return in `PlayerMoveEvent`, set an explicit `EventPriority`).

Be specific: name the symptom ("holds `Player` in `Map<String,Player> homes` — pins the object
after logout") and the fix ("key by `UUID`, resolve via `Bukkit.getPlayer(uuid)`"). If a category
is clean, say so in one line rather than padding.

End with a short **summary**: counts per severity, the top 1–3 things to fix first, and a pointer
to re-run `/build` and `/run-server` after fixing. This skill recommends fixes; it does not apply
them — offer to hand off to the relevant implement skill (`/add-command`, `/add-listener`, …) or
to make the edits in a follow-up.