Skip to content

fix: limit /tfmc parrot to Ascended and match fishing tutorial items - #31

Merged
ryanbarlow97 merged 1 commit into
mainfrom
fix/tfmc-parrot-fishing
Sep 28, 2026
Merged

ryanbarlow97 merged 1 commit into
mainfrom
fix/tfmc-parrot-fishing

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #30. Both changes are in the bundled tfmc.yml; nothing else changes.

  • /tfmc parrot is Ascended-only. parrot.permissions is now [group.ascended], because the command grants flight. The Legacy group also holds group.ascended, so both tiers get it. I checked this on dev with lp group legacy permission check group.ascended. Anyone who isn't Ascended no longer sees parrot. unparrot stays visible to everyone.

  • The fishing tutorial clear only takes back the tutorial's items. It used to run clear <player> shears 1 and clear <player> paper 1, which remove any shears or paper. The Iron Hook and Fish Bait are CustomFishing items built on those materials, tagged minecraft:custom_data → CustomFishing.id. I confirmed the tag in Main player data. The clear now uses item predicates:

    • minecraft:shears[minecraft:custom_data~{CustomFishing:{id:"iron_hook"}}]
    • minecraft:paper[minecraft:custom_data~{CustomFishing:{id:"fish_bait"}}]

    Both parse on TFMCDev01, where a control with an unknown component is rejected. The rod line is unchanged: CustomFishing's fishing_rod is the MMOItems FISHING_RODS:FISHING_ROD.

TFMCDev01's own tfmc.yml has the same edits, applied with /tcore reload tfmc. mvn verify: 218 tests pass, and they now cover parrot gating and the exact fishing clear commands.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fishing tutorial cleanup now removes only shears and paper marked as fishing items, leaving ordinary shears and paper untouched.
    • The parrot command is hidden from ordinary players and remains available to Ascended players.

- parrot grants flight, so it now needs group.ascended (Legacy holds it
  too) instead of being open to everyone
- the fishing tutorial clear removed any shears and paper; the Iron Hook
  and Fish Bait are CustomFishing items built on those, so match their
  CustomFishing id in custom_data and leave ordinary items alone

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 28, 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: ba4d68f9-4077-4818-96a1-2acd9fc73fff

📥 Commits

Reviewing files that changed from the base of the PR and between 4087608 and d14714d.

📒 Files selected for processing (2)
  • src/main/resources/tfmc.yml
  • src/test/java/net/tfminecraft/tfmccore/tfmc/TfmcCommandTest.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 fishing tutorial cleanup now matches CustomFishing item IDs before removing items. The parrot command now requires Ascended permissions. Tests reflect the updated cleanup checks, command visibility, and parrot-flight player permissions.

Changes

Command configuration updates

Layer / File(s) Summary
Fishing tutorial cleanup
src/main/resources/tfmc.yml, src/test/java/net/tfminecraft/tfmccore/tfmc/TfmcCommandTest.java
Fishing cleanup targets shears with ID iron_hook and paper with ID fish_bait. The tutorial-clear test resets captured console commands before checking removal commands.
Parrot command access and tests
src/main/resources/tfmc.yml, src/test/java/net/tfminecraft/tfmccore/tfmc/TfmcCommandTest.java
The parrot permission list includes group.ascended. Tests exclude parrot for ordinary players and use Ascended players in parrot-flight scenarios.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to d1471

The change narrows fishing cleanup to tagged items and gates parrot access on group.ascended. No actionable regression is established in the available source, so the PR appears mergeable; live plugin item data and group inheritance remain unverified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d1471

The flight permission is more restrictive, and no new unauthorized route to flight was found. The narrower fishing cleanup should spare ordinary items, but its effectiveness depends on the live tutorial items carrying the expected tags; the command tests do not verify actual removal.

Retained concerns

  • Low · architecture · inferred: The narrower fishing cleanup relies on exact CustomFishing item data that the command-string tests do not verify. If live items differ, the command can report clearance while leaving tutorial items in the player's inventory; a mismatch has not been established.
Security review details

Security Blast Radius

  • inferred — The changed flight gate affects players invoking this TFMC command when enabled; fishing predicates affect items targeted by that tutorial's clear action. No cross-service dependency edge or broader production entrypoint change was established.

Trust Boundaries and Controls

  • observed — Player permission is checked at the parrot command node before flight begins. Authorization is not rechecked for an active flight, which remains subject to its existing expiration and cleanup paths.

Resilience and Maintainability Implications

  • observed — Permission loss does not remove access to unparrot while the feature stays enabled, and quit and scheduled expiration also terminate tracked flight. The tests exercise these paths with mocked actions, not live permission changes.

Hardening Proposals

  • proposed — Before enabling these changes in production, verify removal against actual Iron Hook and Fish Bait items and check effective Ascended and Legacy permissions after reload. These checks would resolve external-contract uncertainty, not address a demonstrated bypass.
🚥 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 4 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 and concisely identifies both main changes: restricting /tfmc parrot to Ascended and matching fishing tutorial items.
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 4 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 hook and bait
No plain old paper meets its fate
The parrot waits for Ascended flight
The tests now track the commands right
I nibble, hop, and call it neat!

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

@ryanbarlow97
ryanbarlow97 merged commit 71e2118 into main Sep 28, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the fix/tfmc-parrot-fishing branch September 28, 2026 19:27
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