Skip to content

Avoid direct dependency on shaded EntityTaskResult - #1679

Merged
BenCodez merged 17 commits into
masterfrom
fix/eclipse-entity-task-result-resolution
Sep 29, 2026
Merged

BenCodez merged 17 commits into
masterfrom
fix/eclipse-entity-task-result-resolution

Conversation

@BenCodez

@BenCodez BenCodez commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • stop naming SimpleAPI's shaded FoliaLib EntityTaskResult directly in VotingPlugin production code
  • treat the completion value as an opaque enum and check the stable SUCCESS enum name
  • update scheduler-related tests to use local compatibility statuses instead of importing the relocated FoliaLib enum

Why

SimpleAPI relocates FoliaLib during shading. Eclipse/m2e can resolve SimpleAPI differently from Maven (especially with workspace projects / SNAPSHOT artifacts), which makes com.bencodez.simpleapi.folialib.enums.EntityTaskResult fail to resolve even though Maven packaging sees the shaded class.

This keeps the runtime behavior unchanged while removing the fragile compile-time dependency on that relocated enum class.

Validation

  • verified the branch contains no direct references to com.bencodez.simpleapi.folialib.enums.EntityTaskResult
  • local Maven execution could not be run from this environment because outbound GitHub/DNS access is unavailable; CI should provide the build validation

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of scheduled task outcomes. Successful tasks are recognized correctly, while failed or unavailable tasks continue to trigger the appropriate rejection or fallback behavior for voting point transfers and VoteShop purchases.
    • Updated task handling to work consistently across supported server scheduling environments.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T02:28:54.056340Z 7985674 PR opened
🔒 Security Review ✅ Completed 2026-09-29T02:28:58.941787Z 7985674 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.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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: 546d7701-2fe3-45cd-a7e7-2485cddaef1e

📥 Commits

Reviewing files that changed from the base of the PR and between d6005b6 and 7985674.

📒 Files selected for processing (11)
  • VotingPlugin/src/main/java/com/bencodez/votingplugin/user/SharedMysqlPointMutator.java
  • VotingPlugin/src/main/java/com/bencodez/votingplugin/util/BukkitCompletionScheduler.java
  • VotingPlugin/src/main/java/com/bencodez/votingplugin/voteshop/service/VoteShopPurchaseService.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/commands/CommandLoaderSchedulingTest.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/placeholders/PlaceHoldersWorkerSafetyTest.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/backgroundtask/VotingPluginBackgroundTaskTest.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/reminders/VoteRemindersManagerTest.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/user/VotingPluginUserPointSchedulingTest.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/util/BukkitCompletionSchedulerTest.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/util/EntityTaskResultTestCompat.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/voteshop/service/VoteShopPurchaseServiceTest.java

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

📜 Recent review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: build
  • GitHub Check: Analyze (java-kotlin)

📝 Walkthrough

Walkthrough

The Folia completion path now classifies task results by checking whether the status is an enum named SUCCESS. Both point-transfer paths use this helper. Tests use a compatibility helper for opaque success, retirement, and pending results.

Changes

Folia task result handling

Layer / File(s) Summary
Shared status classification
VotingPlugin/src/main/java/com/bencodez/votingplugin/util/BukkitCompletionScheduler.java, VotingPlugin/src/main/java/com/bencodez/votingplugin/user/SharedMysqlPointMutator.java, VotingPlugin/src/main/java/com/bencodez/votingplugin/voteshop/service/VoteShopPurchaseService.java
The completion scheduler checks whether a status is an enum named SUCCESS. Both entity-task callbacks use that check and retain their fallback or rejection behavior.
Test status fixtures and scheduler scenarios
VotingPlugin/src/test/java/com/bencodez/votingplugin/util/EntityTaskResultTestCompat.java, VotingPlugin/src/test/java/com/bencodez/votingplugin/...
A test helper supplies success, retirement, and pending futures. Scheduler tests use these fixtures instead of constructing futures with EntityTaskResult values.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to 79856

Folia task completion preserves the existing success and fallback behavior, and the reviewed changes show no concrete merge-blocking regression.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 79856

Normal success and retirement handling appears preserved, but the new success rule accepts any enum named SUCCESS. The scheduler supplies production results, and its exact result contract has not been verified.

Retained concerns

  • Low · architecture · inferred: The new success contract does not verify the result enum's origin. If the Folia adapter can return a different enum whose SUCCESS value does not mean that the entity task was admitted, transfer or purchase rejection could be suppressed. That producer behavior is unverified, not an observed attack path.
Security review details

Security Blast Radius

  • inferred — A misclassified completion could affect the fallback decision for a scheduled task, including transfer approval or a purchase step. The inspected code does not establish that a player can choose the completion status.

Trust Boundaries and Controls

  • observed — The Folia adapter supplies the completion value. Non-SUCCESS names and exceptional completion take fallback or rejection paths; callback state guards constrain repeated rejection or execution.

Resilience and Maintainability Implications

  • inferred — For the documented success and retirement statuses, the classifier preserves the intended branch selection. The purchase and transfer state fences limit duplicate callbacks, but their end-to-end behavior cannot be established for an incompatible status from the external producer.

Hardening Proposals

  • proposed — Verify and document the Folia adapter's completion-status contract, particularly whether any distinct SUCCESS enum can represent a result that did not admit or execute the entity task.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 11 files. 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: removing direct dependencies on the shaded EntityTaskResult type.
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.
  • 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

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

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

@BenCodez
BenCodez merged commit 33109b1 into master Sep 29, 2026
6 checks passed
@BenCodez
BenCodez deleted the fix/eclipse-entity-task-result-resolution branch September 29, 2026 02:37
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