Clear the Discord Banned role when a ban ends

[XxFran10xX]

Fixes the bug report "banned people on Discord keep the role after their ban is lifted".

Cause

The in-game → Discord ban mirror never ran. It listened for Essentials' BanStatusChangeEvent, which EssentialsX 2.22 does not have, so every startup on Main logged:

[TFMCWeb] Essentials present but BanStatusChangeEvent missing — ban mirror disabled.

The ProvinceSystem moderation outbox has never held a ban or unban row. Staff added the Banned role by hand with /minecraftban, and nothing ever removed it. Timed bans also expire without any event, so they could not have cleared the role even if the listener had registered.

Change

• Replace EssentialsBanListener with BanMirrorPoller. It reads the server's profile ban list every 30 s (/ban, /tempban, /unban and /pardon all write to it) and posts the differences to /skins/moderation/ban-events:
  • new entry → ban, with the reason, duration and source (the bot sends the DM and adds the role)
  • removed entry → unban
  • a timed ban past its expiry → unban with issuer "Ban expired". The entry may stay in banned-players.json until the player logs in, but it is treated as ended.
• Mirrored bans are stored in plugins/TFMCWeb/ban-mirror.yml, so restarts don't resend them. On the first run the current list is adopted without notifying anyone, so there is no flood of DMs when this is deployed.
• Reasons and names are cleaned to what the API's text checks accept: colour codes are removed and symbols such as | become spaces. Reasons already on Main, like Not using RP chats correctly (3.1) | Gri…, would otherwise be rejected with a 400.
• If a post fails, that change is retried on every poll, with one warning per player until it gets through.
• New config: ban-mirror.enabled (default true) and ban-mirror.poll-seconds (default 30). If the keys are missing, these defaults apply, so existing live configs need no edit.

No bot or ProvinceSystem changes are needed: the bot already removes the Banned role on unban.

Not covered

• Players whose bans ended before this is deployed keep the role. Staff need to remove it by hand once.
• A role added with /minecraftban for someone who was never banned in-game is not tracked.

Testing

• mvn clean verify: 107 tests pass and the 100% line coverage gate is met. New BanMirrorPollerTest covers first-run adoption, restart persistence, ban/unban/expiry/re-ban, API failure retry, overlapping ticks, ban-list errors and text cleaning.
• Dev server test results are posted below.

🤖 Generated with Claude Code

[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: fe29ef7e-dd1e-4b69-8ec1-1fa1b5ae5fc5

📥 Commits

Reviewing files that changed from the base of the PR and between 4bacf2c and 4824812.

📒 Files selected for processing (12)

• README.md
• src/main/java/net/tfminecraft/tfmcweb/Cache.java
• src/main/java/net/tfminecraft/tfmcweb/TFMCWeb.java
• src/main/java/net/tfminecraft/tfmcweb/listeners/EssentialsBanListener.java
• src/main/java/net/tfminecraft/tfmcweb/loaders/ConfigLoader.java
• src/main/java/net/tfminecraft/tfmcweb/managers/BanMirrorPoller.java
• src/main/resources/config.yml
• src/test/java/net/ess3/api/events/BanStatusChangeEvent.java
• src/test/java/net/tfminecraft/tfmcweb/TFMCWebLifecycleTest.java
• src/test/java/net/tfminecraft/tfmcweb/listeners/EssentialsBanListenerTest.java
• src/test/java/net/tfminecraft/tfmcweb/loaders/ConfigurationTest.java
• src/test/java/net/tfminecraft/tfmcweb/managers/BanMirrorPollerTest.java

💤 Files with no reviewable changes (3)

• src/test/java/net/tfminecraft/tfmcweb/listeners/EssentialsBanListenerTest.java
• src/test/java/net/ess3/api/events/BanStatusChangeEvent.java
• src/main/java/net/tfminecraft/tfmcweb/listeners/EssentialsBanListener.java

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

---

📝 Summary

Summary by CodeRabbit

• New Features
  • Ban and unban changes from the server ban list are now mirrored to linked services, including timed bans that expire.
  • Ban mirroring is enabled by default and can be disabled or configured with a polling interval. Changes apply after a restart.
  • Existing bans are recorded when mirroring starts without sending them as new events. Failed updates are retried on subsequent polls.

Walkthrough

The plugin replaces Essentials ban-event handling with polling of the profile ban list. It sends detected bans, unbans and expired timed bans to ProvinceSystem. Configuration controls the polling interval and whether mirroring is enabled.

Changes

Ban-list moderation mirroring

Layer / File(s)	Summary
Configure and wire the poller src/main/java/net/tfminecraft/tfmcweb/Cache.java, src/main/java/net/tfminecraft/tfmcweb/loaders/ConfigLoader.java, src/main/resources/config.yml, src/main/java/net/tfminecraft/tfmcweb/TFMCWeb.java, src/main/java/net/tfminecraft/tfmcweb/listeners/EssentialsBanListener.java, src/test/java/net/ess3/api/events/BanStatusChangeEvent.java, src/test/java/net/tfminecraft/tfmcweb/TFMCWebLifecycleTest.java, src/test/java/net/tfminecraft/tfmcweb/listeners/EssentialsBanListenerTest.java, src/test/java/net/tfminecraft/tfmcweb/loaders/ConfigurationTest.java, README.md	Configuration adds an enabled-by-default ban mirror and a 30-second default poll interval. The plugin starts and stops BanMirrorPoller instead of registering EssentialsBanListener. The README now describes ban-list events, including expired timed bans. Lifecycle and configuration tests cover the new wiring and settings.
Schedule polling and detect changes src/main/java/net/tfminecraft/tfmcweb/managers/BanMirrorPoller.java	The poller adopts its initial snapshot without posting events. Later polls detect new, updated, removed and expired bans. It prevents overlapping polls and persists known records to ban-mirror.yml.
Resolve identity and post moderation events src/main/java/net/tfminecraft/tfmcweb/managers/BanMirrorPoller.java, src/test/java/net/tfminecraft/tfmcweb/managers/BanMirrorPollerTest.java	The poller resolves player identities and posts ban or unban data. It sanitises names and reasons, formats durations, and retries failed posts on later polls. Tests cover posting, retries, state handling and text formatting.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant BanMirrorPoller
  participant ProfileBanList
  participant LinkCache
  participant ProvinceSystemClient
  participant BanMirrorFile
  BanMirrorPoller->>ProfileBanList: Read current profile bans
  ProfileBanList-->>BanMirrorPoller: Return ban records
  BanMirrorPoller->>LinkCache: Check for a cached player identity
  LinkCache-->>BanMirrorPoller: Return cached identity when available
  BanMirrorPoller->>ProvinceSystemClient: Resolve identity or post moderation event
  ProvinceSystemClient-->>BanMirrorPoller: Return identity or API result
  BanMirrorPoller->>BanMirrorFile: Save known ban state

Loading

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 48248

No actionable issue is established that would prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 48248

The change improves ban-expiry synchronization, but a failed state save can leave Discord moderation out of sync after a restart. Account-link reconciliation and duplicate-delivery behavior also depend on guarantees not available for review.

Retained concerns

• Medium · reliability · inferred: Delivery checkpoints are not recovered after persistence failure. A successful ban POST updates known, but an unsuccessful state save is not retried while snapshots remain unchanged. If the durable checkpoint still lacks that ban and the process restarts after its expiry or removal, both the loaded checkpoint and current snapshot can omit it, so no unban is emitted. This can strand the downstream Banned role unless an external reconciler repairs it. Stale checkpoints can also cause acknowledged transitions to be submitted again; their downstream effects depend on an unavailable idempotency contract.

Security review details

Security Blast Radius

• inferred — The newly active producer can forward changes for every UUID-bearing profile ban on this server, including expiry and removal, to the configured moderation service. Intended effects include Discord DMs and Banned-role changes. The receiver's guild, tenant, environment and permission scope cannot be determined from this client repository.

Trust Boundaries and Controls

• observed — The moderation subject comes from a server profile UUID, not from a newly exposed player-facing request. The centralized client sends X-Plugin-Key to the configured endpoint with connect and read timeouts. Identity responses and cached IDs are trusted locally; validation of UUID-to-Discord binding and server authority belongs to an unavailable downstream implementation.

Resilience and Maintainability Implications

• inferred — Remote delivery and local checkpoint persistence are separate operations. Ordinary API failures are retryable, but an acknowledged transition can survive only in memory after a save failure. Restart recovery therefore does not guarantee continued alignment of moderation state, and replay safety depends on the receiver's duplicate-handling contract.

Hardening Proposals

• proposed — Make checkpoint failure a recoverable state: retain a dirty checkpoint for independent retry, distinguish failed recovery from intentional first-run adoption, and define replay-safe delivery with the receiver. A durable pending-transition mechanism could preserve required unbans across interruption.
• proposed — Establish an explicit consumer contract for identity binding, server and environment ownership, duplicate ordering, and reconciliation when an accepted event was not mirrored or an account link changes. These are validation proposals, not verified downstream vulnerabilities.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[XxFran10xX]

Dev test (TFMCDev01, dev API on :18001)

Tested with a linked test account. Dev's outbox is not polled by the live bot. The test rows were marked delivered afterwards.

Step	Console	Outbox row
Startup	—	Ban mirror polling the ban list every 30s. (the old BanStatusChangeEvent missing warning is gone)
First poll, with a 1 m tempban already in place	tempban … 1m …	none; the ban was adopted silently into ban-mirror.yml
That ban expires	—	unban, staff_name: "Ban expired", 6 s after expiry
New ban with a `	` in the reason	tempban … 2m (3.1) | Ban mirror test…
It expires	—	unban, Ban expired
7 day ban (after the duration fix)	tempban … 7d …	ban, duration 7 days
Manual unban	unban …	unban, staff null (bot shows "In-game")

The 2 m ban was first reported as 1m (time left, not ban length). This is fixed in 046e018, and the 7 day ban reports 7 days.

🤖 Generated with Claude Code

[XxFran10xX]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.