fix: load after RPCharacters by dropping loadbefore ItemsAdder

[ryanbarlow97]

Why

On 2026-09-29, Main was started without -Dpaper.useLegacyPluginLoading=true and crash-looped:

Circular plugin loading from plugins RPCharacters
RPCharacters -> TLibs -> ItemsAdder -> BirdMessenger -> RPCharacters

BirdMessenger depends on TLibs, which soft-depends on ItemsAdder, so loadbefore: [ItemsAdder] could never hold. The startup logs on both servers show ItemsAdder already enabling before BirdMessenger.

The legacy loader broke the cycle at BirdMessenger's soft dependency instead, so RPCharacters enabled after BirdMessenger. As a result, isPluginEnabled("RPCharacters") in onEnable returned false, and CharacterActivatedListener never registered. Pending mail was not delivered when a player switched to the addressed character.

Changes

• Removed loadbefore: [ItemsAdder] from plugin.yml and added a comment explaining why it must stay out.
• Removed the incorrect "ItemsAdder is loaded after us" comment.
• Added PluginLoadOrderTest, which fails if a loadbefore returns or a dependency is dropped.

Remaining cycle

Across Main's 73 plugins, this change removes 13 of the 14 load-order cycles. The remaining one is RPCharacters ↔ SimpleFactions: each soft-depends on the other. Main will still need the legacy loading flag until one of those two edges is removed.

Testing

mvn verify passes: 34 tests, 0 failures.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Compatibility
  • Updated plugin startup ordering for compatibility with related integrations. The required TLibs dependency remains unchanged, and ItemsAdder and RPCharacters remain optional integrations.
• Tests
  • Added a check to verify the plugin’s dependency and startup-order settings.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 2678edbf-98d6-444f-9a46-09090a525f8b

📥 Commits

Reviewing files that changed from the base of the PR and between f7f114d and 842bedf.

📒 Files selected for processing (3)

• src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java
• src/main/resources/plugin.yml
• src/test/java/net/tfminecraft/birdmessenger/PluginLoadOrderTest.java

💤 Files with no reviewable changes (1)

• src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The plugin descriptor removes loadbefore: [ItemsAdder] and adds comments about the load-order cycle. The existing TLibs dependency remains. A new test checks the descriptor’s dependency declarations.

Changes

Plugin load order

Layer / File(s)	Summary
Update and check load-order declarations src/main/resources/plugin.yml, src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java, src/test/java/net/tfminecraft/birdmessenger/PluginLoadOrderTest.java	The descriptor removes the ItemsAdder load-before declaration and adds comments about the load-order cycle. The listener registration remains unchanged. The new test checks the dependency declarations.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 842be

The load-order declaration changes, but the inspected code establishes no concrete interaction failure. No specific merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 842be

The startup change is intended to restore character-activation mail delivery. That newly effective delivery path relies on an unverified assumption that character IDs cannot belong to different players, and its behavior under the remaining startup cycle has not been demonstrated.

Retained concerns

• Medium · security · inferred: When the changed startup order permits character-activation delivery, pending mail is retrieved by character ID without checking the stored owner UUID. If RPCharacters permits the same ID for different owners, an activating player could receive another player’s mail. ID reuse and effective production ordering remain unverified; the underlying check was not introduced by this PR.

Security review details

Security Blast Radius

• inferred — The potential exposure is pending letters and their items for players on a server, conditional on cross-owner character-ID reuse. The changed descriptor shows no new command permission or authority grant.

Security Findings and Attack Paths

• inferred — If two players can activate characters with the same ID, activation-triggered delivery can take all pending entries for that ID and give them to the activating player without checking each recorded owner. The available evidence does not establish that such an ID collision is possible.

Trust Boundaries and Controls

• observed — The active-character check binds the supplied ID to the activating player, and submission validates an owner-and-character pair. Neither check compares the activating player with the owner stored on each pending letter at consumption time.

Resilience and Maintainability Implications

• inferred — A crash after pending entries are saved as removed but before item grant can lose mail. This is an existing failure window that becomes relevant to the intended activation-delivery flow, not a persistence change made by the PR.

Hardening Proposals

• proposed — Confirm RPCharacters ID uniqueness or bind pending-mail lookup and delivery to both owner UUID and character ID; exercise a cross-owner ID collision in a test.
• proposed — Check listener registration and pending-mail delivery during a server startup using the intended loader configuration, including recovery after an interrupted delivery.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: removing loadbefore: [ItemsAdder] to adjust plugin load order relative to RPCharacters.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{A rabbit checks the plugin’s order,
Then nibbles clover by the border.
No load-before entry stays,
The test checks dependencies’ ways.
The listener waits, unchanged in place,
And bunny hops off with a grin on its face.}

_{Comment @coderabbitai help to get the list of available commands.}