Skip to content

fix: reject mail to recipients who opted out before confirmation - #26

Merged
ryanbarlow97 merged 1 commit into
mainfrom
feat/mail-recipient-toggle
Sep 25, 2026
Merged

ryanbarlow97 merged 1 commit into
mainfrom
feat/mail-recipient-toggle

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

If a character opts out of receiving mail after a sender opens the recipient picker, confirmation now returns the letter instead of sending it to the stale selection. The check matches both owner UUID and character ID against RPCharacters' current recipient list.

Adds a configurable rejection message. Existing in-flight and pending deliveries remain unchanged. The per-character toggle and persistence live in TF-Minecraft/RPCharacters#49; deploy both changes for the complete behavior. This uses the existing RPCharacters API.

Validation: Java 21, mvn -o -q verify passed (26 tests). New tests verify that unavailable recipients get the letter returned without queueing delivery, and listed recipients still reach normal flight validation. Adds JSON Simple in test scope to initialize the RPCharacters API in Mockito. No live-server test performed.

Summary by CodeRabbit

  • Features
    • Players can opt out of receiving mail for their active character and restore mail access with the documented commands.
    • If a recipient opts out before a letter is confirmed, the letter is returned to the sender. Mail already sent is still delivered.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: c2216ed3-5880-447b-9018-95a68ccff96c

📥 Commits

Reviewing files that changed from the base of the PR and between cdf9935 and b413411.

📒 Files selected for processing (6)
  • README.md
  • pom.xml
  • src/main/java/net/tfminecraft/birdmessenger/BirdConfig.java
  • src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java
  • src/main/resources/config.yml
  • src/test/java/net/tfminecraft/birdmessenger/mail/MailRecipientOptOutTest.java

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


📝 Walkthrough

Walkthrough

MailService now checks that the selected character remains available for mail before calculating flight time. If no listed mail target matches the recipient’s owner UUID and character ID, it returns the letter and sends an unavailable message.

Changes

Recipient Mail Check

Layer / File(s) Summary
Validate recipient before sending
src/main/java/net/tfminecraft/birdmessenger/BirdConfig.java, src/main/java/net/tfminecraft/birdmessenger/mail/MailService.java, src/main/resources/config.yml, src/test/java/net/tfminecraft/birdmessenger/mail/MailRecipientOptOutTest.java, README.md, pom.xml
MailService.trySend checks the listed mail targets before flight-time calculation. If the recipient is absent, it returns the letter and sends the configured unavailable message. Tests cover the unavailable and listed-recipient paths. The README documents recipient opt-out behavior. The build adds a test-scoped json-simple dependency.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b4134

The change is ready to merge after normal checks. Its behavior if RPCharacters is disabled during an open confirmation remains unverified, but no resulting failure has been established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b4134

The new check reduces mail sent to characters who opted out before confirmation. Its effect depends on the recipient service being deployed alongside it, and unexpected recipient data could interrupt the letter-return path. No broader access or privilege change is evident.

Retained concerns

  • Low · reliability · inferred: If the recipient list contains a null entry, the new confirmation check throws before returning the letter; the caller has already closed the picker and has no exception cleanup. The provider contract does not establish whether this input can occur.
Security review details

Security Blast Radius

  • inferred — The changed decision concerns whether an individual character can receive a sender’s letter. The visible paths do not add a tenant-wide authority, credential, infrastructure, or network boundary.

Security Findings and Attack Paths

  • inferred — No attacker-controlled path through the new check to unauthorized delivery is established: an absent owner-and-character match refuses the send. Provider nullability and concurrent mutation remain unverified, so the check is not proof of an atomic opt-out guarantee.

Trust Boundaries and Controls

  • observed — A previously selected target is rechecked against RPCharacters before flight validation and storage. This moves the recipient-availability decision from picker-time alone to confirmation-time as well.

Resilience and Maintainability Implications

  • inferred — An exception in the new provider-list traversal can interrupt item ownership cleanup: the picker is closed with confirmation marked true, while refusal cleanup runs only on a false return.

Hardening Proposals

  • proposed — Align confirmation’s handling of malformed provider entries with the picker and ensure an exception still settles letter ownership. Establish the provider’s list snapshot and mutation guarantees before claiming that a concurrent opt-out cannot race the enqueue.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (3 skipped: 3… 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: rejecting mail when recipients opt out before confirmation.
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 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (3 skipped: 3 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 mailbound trail
No listed name? Back comes the mail
A gentle note goes hopping through
The waiting sender hears what’s true
Then off I nibble, pleased and pale

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

@ryanbarlow97
ryanbarlow97 merged commit 6a3d8d7 into main Sep 25, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the feat/mail-recipient-toggle branch September 25, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant