fix: deliver pending mail on character switch when RPCharacters enables late

[ryanbarlow97]

Why

BirdMessenger.onEnable registered CharacterActivatedListener only when RPCharacters was already enabled. On both servers, the legacy plugin loader enables RPCharacters after BirdMessenger, because the RPCharacters ↔ SimpleFactions soft-dependency cycle makes it ignore soft dependencies. So the listener never registered, and pending mail was not delivered when a player switched to the addressed character.

#29 removed BirdMessenger's own cycle. After dev restarted on v1.1.4, it still showed:

16:54:23  Enabling BirdMessenger v1.1.4
16:54:24  Enabling RPCharacters v2.10.1

Changes

• Added RpCharactersHook. It registers CharacterActivatedListener at startup if RPCharacters is already enabled, or when PluginEnableEvent fires for RPCharacters, and never twice. SimpleFactions and RPCharacters already handle each other the same way.
• RpCharactersHookTest covers three cases: RPCharacters enabling late, RPCharacters already enabled with no double registration, and other plugins being ignored.

Testing

mvn verify passes: 37 tests, 0 failures.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Character-activated mail handling now registers whether RPCharacters is already enabled at startup or enabled later.
  • Prevented duplicate registration when RPCharacters is detected through multiple startup events.

[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: 4a8bf345-a2d8-4605-a9ff-b5e274e53b57

📥 Commits

Reviewing files that changed from the base of the PR and between 597cd95 and b82e3eb.

📒 Files selected for processing (4)

• src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java
• src/main/java/net/tfminecraft/birdmessenger/listener/RpCharactersHook.java
• src/test/java/net/tfminecraft/birdmessenger/PluginLoadOrderTest.java
• src/test/java/net/tfminecraft/birdmessenger/listener/RpCharactersHookTest.java

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

---

📝 Walkthrough

Walkthrough

BirdMessenger now registers a hook at startup. The hook registers CharacterActivatedListener if RPCharacters is enabled or when RPCharacters later enables. Tests cover both registration paths and ignore enable events from other plugins.

Changes

RPCharacters Registration

Layer / File(s)	Summary
Hook and registration behavior src/main/java/net/tfminecraft/birdmessenger/listener/RpCharactersHook.java, src/test/java/net/tfminecraft/birdmessenger/listener/RpCharactersHookTest.java	The hook checks whether RPCharacters is enabled and registers CharacterActivatedListener once. Tests cover registration after enablement, an already-enabled plugin, and an unrelated plugin's enable event.
Startup wiring src/main/java/net/tfminecraft/birdmessenger/BirdMessenger.java, src/test/java/net/tfminecraft/birdmessenger/PluginLoadOrderTest.java	BirdMessenger registers the hook at startup and calls registerIfEnabled(). The load-order test comment no longer states that RPCharacters must enable first.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b82e3

This change makes pending mail delivery on character switch work when RPCharacters enables after BirdMessenger. The listener is registered exactly once. No merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to b82e3

Late plugin startup should now allow pending mail to be delivered, but the newly reachable delivery path relies on character IDs to select mail without independently checking the stored recipient. Whether that permits delivery to another player depends on a character-ID uniqueness guarantee that has not been established.

Retained concerns

• Medium · security · inferred: Late-enable registration makes activation delivery reachable in the reported load order, while pending mail is selected by character ID without comparing its stored owner UUID to the event owner. Cross-owner delivery is possible if RPCharacters IDs are not globally unique; that uniqueness contract is unverified.

Security review details

Security Blast Radius

• inferred — The added reachability is limited to BirdMessenger's character-activation delivery on servers where RPCharacters enables later. A cross-player exposure would require colliding character IDs; no broader service or environment exposure is shown.

Security Findings and Attack Paths

• inferred — If two owners can have the same character ID, activation by one owner could consume pending letters stored for the other. No ID collision or exploit is established; the listener and owner-check gap predate this PR in the load order where registration already succeeded.

Trust Boundaries and Controls

• observed — The hook requires RPCharacters to be enabled, and delivery requires the supplied character ID to match the event owner's active character. Neither control compares that owner with the pending record's owner UUID.

Resilience and Maintainability Implications

• observed — Successful serial registration is guarded against duplication, while the pre-existing remove-before-deliver sequence lacks an inspected acknowledgement or requeue on delivery failure.

Hardening Proposals

• proposed — Establish and document global character-ID uniqueness, or bind pending-mail lookup and consumption to both the owner UUID and character ID.
• proposed — Consider a recoverable claim-and-acknowledgement transition for pending delivery so an interruption does not silently discard or repeat letters.

🚥 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 11 functions across 4 files.	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 and concisely describes the main change: delivering pending mail on character switches when RPCharacters enables after BirdMessenger starts.
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.

• 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 checked the plugin's state,
And watched for RPCharacters to wake.
If it was ready, one listener joined,
If it woke later, the hook did the same.
Other plugin bells rang by,
While the rabbit munched a carrot nearby.}

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