Skip to content

refactor: remove dev-characters and the dev character tag - #59

Merged
ryanbarlow97 merged 1 commit into
mainfrom
refactor/remove-dev-characters
Sep 27, 2026
Merged

ryanbarlow97 merged 1 commit into
mainfrom
refactor/remove-dev-characters

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Why

dev-characters was a pre-season tool. It stopped a server syncing characters with its website, and tagged characters made in game with "dev": "true". Staff tests on Dev then stayed off the main site, and /rpcharacter wipe tagged could delete them.

Since 23 September Dev has had its own dev website (dev.tfminecraft.net, realm dev), so that isolation isn't needed any more.

Changes

Removed:

  • the dev-characters setting and its Cache field;
  • the dev field on RPCharacter, and reading and writing it in character files;
  • tagging new characters in CharacterCreation;
  • the dev-characters guards in CharacterIngestService, KitCustomiseIngestService, KitService and RosterSyncService, and the /rpcharacter pending sync refusal;
  • /rpcharacter wipe tagged and CharacterWipeService, which only served it, along with its test. /rpcharacter wipe website is unchanged apart from dropping the per-action confirm bookkeeping it no longer needs.

Every server now pulls pending creates and kit customisations, and pushes its full roster to its own site. Existing "dev" keys in character files are ignored.

Effect on servers

  • Main: runs with dev-characters: false and has no tagged characters (0 of 240), so nothing changes.
  • Dev: switched to dev-characters: false earlier today, and Wonder, its only tagged character, was untagged. So nothing changes there either.

The leftover dev-characters: line in either server's config.yml is now unused and harmless.

Docs: TF-Minecraft/Docs#70, which renames the page to the website realm wipe.

Testing

  • mvn verify: 285 tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Changes
    • Removed the development-character setting and related character tagging. Character pulls, roster sync, and kit processing now run without being blocked by that setting, and development characters are included in roster synchronization.
    • The /rpcharacter wipe command now supports only the website wipe action; tagged-character wipe is no longer available. Tab completion reflects the remaining action.
    • Character records no longer store or load a development-character flag.

dev-characters kept a server from syncing characters with its website
and tagged characters made in game as "dev", so pre-season staff tests
stayed off the main site and could be deleted with /rpcharacter wipe
tagged. Dev now has its own dev website with realm dev, so that
isolation is no longer needed.

Remove the setting, the tag and the tagged wipe. Every server now pulls
pending creates and kit customisations and pushes its full roster to
its own site. The website realm wipe is unchanged. Existing "dev" keys
in character files are ignored.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 27, 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: 26842c3e-fcc8-411e-9f51-87954721bb0c

📥 Commits

Reviewing files that changed from the base of the PR and between 3b737cb and f8e8806.

📒 Files selected for processing (15)
  • src/main/java/net/tfminecraft/rpcharacters/Cache.java
  • src/main/java/net/tfminecraft/rpcharacters/creation/CharacterCreation.java
  • src/main/java/net/tfminecraft/rpcharacters/database/Database.java
  • src/main/java/net/tfminecraft/rpcharacters/ingest/CharacterIngestService.java
  • src/main/java/net/tfminecraft/rpcharacters/ingest/KitCustomiseIngestService.java
  • src/main/java/net/tfminecraft/rpcharacters/ingest/RosterSyncService.java
  • src/main/java/net/tfminecraft/rpcharacters/kit/KitService.java
  • src/main/java/net/tfminecraft/rpcharacters/loaders/ConfigLoader.java
  • src/main/java/net/tfminecraft/rpcharacters/managers/CommandManager.java
  • src/main/java/net/tfminecraft/rpcharacters/objects/RPCharacter.java
  • src/main/java/net/tfminecraft/rpcharacters/utils/CommandTabCompleter.java
  • src/main/java/net/tfminecraft/rpcharacters/wipe/CharacterWipeService.java
  • src/main/java/net/tfminecraft/rpcharacters/wipe/WipeCommand.java
  • src/main/resources/config.yml
  • src/test/java/net/tfminecraft/rpcharacters/wipe/CharacterWipeDeletionTest.java
💤 Files with no reviewable changes (9)
  • src/main/java/net/tfminecraft/rpcharacters/loaders/ConfigLoader.java
  • src/main/java/net/tfminecraft/rpcharacters/creation/CharacterCreation.java
  • src/main/java/net/tfminecraft/rpcharacters/database/Database.java
  • src/main/java/net/tfminecraft/rpcharacters/managers/CommandManager.java
  • src/test/java/net/tfminecraft/rpcharacters/wipe/CharacterWipeDeletionTest.java
  • src/main/java/net/tfminecraft/rpcharacters/Cache.java
  • src/main/resources/config.yml
  • src/main/java/net/tfminecraft/rpcharacters/wipe/CharacterWipeService.java
  • src/main/java/net/tfminecraft/rpcharacters/objects/RPCharacter.java

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


📝 Walkthrough

Walkthrough

The change removes the development-character setting and character flag, and removes the tagged-character wipe action. Pull, roster, and kit flows no longer skip processing based on development-character mode.

Changes

Development-character mode removal

Layer / File(s) Summary
Remove development-character state and persistence
src/main/java/net/tfminecraft/rpcharacters/Cache.java, src/main/java/net/tfminecraft/rpcharacters/objects/RPCharacter.java, src/main/resources/config.yml, src/main/java/net/tfminecraft/rpcharacters/loaders/ConfigLoader.java, src/main/java/net/tfminecraft/rpcharacters/creation/CharacterCreation.java, src/main/java/net/tfminecraft/rpcharacters/database/Database.java
The configuration option, character flag, creation tagging, and JSON load/save handling are removed.
Remove development-mode processing guards
src/main/java/net/tfminecraft/rpcharacters/ingest/CharacterIngestService.java, src/main/java/net/tfminecraft/rpcharacters/ingest/KitCustomiseIngestService.java, src/main/java/net/tfminecraft/rpcharacters/ingest/RosterSyncService.java, src/main/java/net/tfminecraft/rpcharacters/kit/KitService.java, src/main/java/net/tfminecraft/rpcharacters/managers/CommandManager.java
Pulls, roster sync, kit-customisation processing, kit claims, and the pending sync command no longer use development-character mode to skip processing.
Remove tagged-character wipe
src/main/java/net/tfminecraft/rpcharacters/wipe/WipeCommand.java, src/main/java/net/tfminecraft/rpcharacters/wipe/CharacterWipeService.java, src/main/java/net/tfminecraft/rpcharacters/utils/CommandTabCompleter.java, src/test/java/net/tfminecraft/rpcharacters/wipe/CharacterWipeDeletionTest.java
The wipe command accepts only the website action. The tagged-character wipe service and its deletion test are removed, and tab completion offers only the website action.

Priority: ➖ Normal

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to f8e88

The change is mergeable after normal checks; no actionable merge-blocking risk was established.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to f8e88

Removing the old isolation mode makes each server depend on its website realm for separation and allows previously tagged character files to enter normal roster sync. The stated rollout reduces the expected exposure, but realm enforcement and the absence of remaining tagged files have not been independently established. The website-wipe confirmation change does not appear to weaken its existing admin check.

Retained concerns

  • Medium · security · inferred: Legacy tagged character files, if any remain outside the stated inventory, are now read as ordinary characters and can be included in website roster pushes. The PR's stated cleanup is important to the security outcome but is not established by repository evidence.
  • Medium · security · inferred: Removing the mode guards makes the external gateway's authentication and realm isolation the decisive boundary for newly eligible pending-create, kit-customisation, and roster operations. This is a dependency and proof gap, not evidence of a cross-realm access flaw; the stated servers already had the guard disabled.
Security review details

Security Blast Radius

  • inferred — Where the removed mode had been enabled, website-supplied pending rows can now reach local character and kit state, and local rosters can reach the website. The maximum independently attackable realm or server scope cannot be determined without gateway enforcement evidence.

Trust Boundaries and Controls

  • observed — Player-scoped kit application matches both character ID and player UUID; roster serialization uses the player's UUID. Server-wide pending pulls rely on the gateway to supply appropriately scoped rows.
  • observed — The website-wipe confirmation displays a realm but stores only a sender expiry. Execution reads the realm again. This target-binding limitation and the remote deletion endpoint predate the PR; accepting only website removes the former alternate-action confirmation path.

Resilience and Maintainability Implications

  • inferred — Local duplicate-create handling supports retry after a lost acknowledgement, and failed kit acknowledgements are logged. Whether remote failed acknowledgements remain retryable, or a partially failed realm wipe is atomic and safe to repeat, depends on unavailable remote behavior.

Hardening Proposals

  • proposed — Before relying on the retired isolation mode, verify the tagged-character inventory across every deployed server and the gateway's per-server authorization and realm scoping for pending reads, acknowledgements, roster writes, and deletion. Document remote retry and partial-failure behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main changes: removing the dev-characters setting and the dev character tag.
  • 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 peeks where dev flags lay,
Then hops through pulls without delay.
No tagged wipe waits by the door,
The website path remains in store.
I nibble clover, pleased and bright,
And bound away beneath the moonlight.

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

@ryanbarlow97
ryanbarlow97 merged commit a610581 into main Sep 27, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the refactor/remove-dev-characters branch September 27, 2026 18:31
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