[BenCodez]

Explicit permission removal leaves a false attachment node, which can override grants from other attachments and retain idle handlers. Unset the node and retire the empty registered attachment while preserving unrelated grants and replacement handlers created by removal callbacks.

Existing permission APIs, persisted state and scheduling contracts are preserved. Offline expiry does not detach live attachments from an unowned context.

Validation:

• 46 focused permission tests pass, including 16 new regressions covering explicit removal, real Bukkit attachment registration, repeated grant/removal, expiry, offline state and replacement callbacks.
• Full producer package: 1,506 unit tests and two artifact checks pass.
• VotingPlugin master: 2,585 unit tests pass, one optional diagnostic is skipped, and five artifact checks pass against the exact candidate SDK.
• MCPerks: 455 tests pass against a private composition with the existing shutdown candidate in Preserve publication ownership through shutdown and live saves #360. Its additional APIs are kept separate from this PR.
• Actual test classpaths, relocated permission instructions, archive integrity and test-class exclusion verified. Fresh independent read-only source review returned no findings.

The VotingPlugin validation JAR is 10,889,344 bytes, above its advisory 10 MiB target; the MCPerks JAR is 9,302,195 bytes. No live server or external permission-provider acceptance is claimed. Hosted checks for this new PR remain pending.

Created with assistance from OpenAI Codex.

Summary by CodeRabbit

• Bug Fixes
  • Removed permissions are now cleared reliably, including timed grants and grants queued while a player is offline.
  • Cleanup preserves unrelated permissions and newer grants, preventing them from being removed accidentally.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a7495696-c152-4296-82ba-5f877a31ec2c

Commits

Reviewing files that changed from the base of the PR and between c264734 and c767067.

Files selected for processing: 4

• AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PermissionHandler.java
• AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PlayerPermissionHandler.java
• AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/ExplicitPermissionRemovalTest.java
• AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/PermissionAttachmentRetirementTest.java

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

Recent review details

Context from checks skipped due to timeout: 3

• GitHub Check: build
• GitHub Check: Analyze (actions)
• GitHub Check: Analyze (java-kotlin)

Additional context used

Learnings: 1

Learning: 2026-10-20T09:10:24.669Z

Learnt from: CR
Repo: BenCodez/AdvancedCore PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-10-20T09:10:24.669Z
Learning: Source excerpt:
# Maintainer and AI-agent guide

## Build and verification

All test classes and test-only helpers must live under `com.bencodez.advancedcore.tests` or its subpackages, with matching directories under `AdvancedCore/src/test/java`. Keep tests out of production packages. Preserve production visibility when relocating tests; use narrowly scoped test-only reflective helpers when package-private access is required. Preserve test discovery and the package-phase artifact tests.

Applied to files:

• AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/PermissionAttachmentRetirementTest.java
• AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/ExplicitPermissionRemovalTest.java

Additional comments: 4

AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PlayerPermissionHandler.java: 1

251-251: LGTM!

AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PermissionHandler.java: 1

335-345: LGTM!

AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/ExplicitPermissionRemovalTest.java: 1

1-190: LGTM!

AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/PermissionAttachmentRetirementTest.java: 1

1-175: LGTM!

Walkthrough

Permission removal now unsets grants and cleans up empty handlers and online attachments. New tests cover live and queued grants, attachment retirement, expiration, and replacement handlers.

Changes: Permission removal lifecycle

Layer / File(s)	Summary
Grant removal and handler cleanup AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PlayerPermissionHandler.java, AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PermissionHandler.java, AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/ExplicitPermissionRemovalTest.java	Permission removal unsets grants. Empty-handler checks and map removals are synchronized, and online attachments are cleared when their handler is removed. Tests cover live and queued grants, unknown permissions, expiration callbacks, and replacement handlers.
Attachment retirement and expiration AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/permissions/PermissionAttachmentRetirementTest.java	Tests cover attachment removal, preservation of other grants, expiration cleanup, offline expiration, replacement grants, and stale-handler cleanup.

Priority: Normal

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

Change: Bug fix

Merge Risk: Minimal · up to c7670

No actionable issue is identified that should prevent merging after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 3.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 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		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the two main changes: unsetting removed permissions and retiring empty attachments.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

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

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	🔄 Running since 2026-10-25T05:00:28.717078Z	c767067	PR opened
🔒 Security Review	✅ Completed 2026-10-25T05:02:36.850510Z	c767067	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.