Skip to content

Complete FoliaLib 0.5.1 player scheduler compatibility - #369

Merged
BenCodez merged 1 commit into
masterfrom
fix/folialib-051-player-admission
Oct 19, 2026
Merged

BenCodez merged 1 commit into
masterfrom
fix/folialib-051-player-admission

Conversation

@BenCodez

@BenCodez BenCodez commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

After the enum import correction, the FoliaLib 0.5.1 upgrade still fails test compilation because the scheduler fixture returns ServerImplementation where getImpl() now returns PlatformScheduler. Update that fixture and use getScheduler() for player dispatch.

FoliaLib can both invoke the retirement fallback and complete the admission future as retired for one task. Share one claim across dispatch, retirement, and rejection so repeated or late signals cannot invoke another callback after work is retained for retry. Match admission enum names without coupling to an original or relocated enum class.

Validation:

  • Reproduced the original three enum compilation errors and five failing scheduler regressions before the correction.
  • 124 focused scheduler/reward/eligibility tests pass.
  • mvn -U -f AdvancedCore/pom.xml clean verify passes: 1,490 regular tests and two packaged-artifact tests.
  • mvn -f AdvancedCore/pom.xml package -Dbuild.number=1282 -P deploy-snapshot passes through packaging; no deployment was run.
  • Actual dependency resolution: AdvancedCore 3.8.2-SNAPSHOT, SimpleAPI 1.0.2-20261019.030111-43, FoliaLib 0.5.1. The final artifact has one relocated FoliaLib namespace; its 20 FoliaLib classes match the resolved SimpleAPI JAR byte for byte. The package test loads its scheduler/enum contract independently of the dependency classpath.

The FoliaLib POM is currently accessible from Nexus and upstream. No dependency uploads or dependency-removal changes are included. VotingPlugin downstream validation also passes with the fixture migration in VotingPlugin #1727: 2,585 regular tests passed, one optional paired-artifact integration test skipped, and all five packaged-artifact tests passed against the exact local AdvancedCore candidate. Its final JAR contains one relocated FoliaLib namespace and the corrected scheduler calls. This PR does not establish a fix for the spectator-mode report.

Summary by CodeRabbit

  • Bug Fixes
    • Improved player reward task handling when the scheduler retires or rejects a task, so retained work is not also run by a late callback.
    • Prevented action, retirement, and rejection callbacks from being triggered more than once, including when scheduling completes exceptionally or returns an unrecognized result.
    • Kept non-Folia task scheduling behavior unchanged.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 3e0026d2-3dbf-463d-9869-1747161af094
📥 Commits

Reviewing files that changed from the base of the PR and between 13dd10e and 49d5369.

📒 Files selected for processing: 4
  • AdvancedCore/src/main/java/com/bencodez/advancedcore/api/rewards/RewardEligibility.java
  • AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/rewards/PlayerTaskAdmissionTest.java
  • AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/rewards/RetiringOwnerScheduler.java
  • AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/packaging/PackagedArtifactTest.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 comments: 4
AdvancedCore/src/main/java/com/bencodez/advancedcore/api/rewards/RewardEligibility.java: 1

12-12: LGTM!

Also applies to: 97-98, 108-126

AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/rewards/PlayerTaskAdmissionTest.java: 1

1-174: LGTM!

AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/rewards/RetiringOwnerScheduler.java: 1

17-17: LGTM!

Also applies to: 38-38, 42-42

AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/packaging/PackagedArtifactTest.java: 1

5-21: LGTM!

Also applies to: 42-78

📝 Walkthrough

Folia task dispatch now uses FoliaLib’s scheduler and an atomic claim to arbitrate action execution, retirement, and rejection. New tests cover admission outcomes, non-Folia scheduling, and the FoliaLib classes in the packaged artifact.

Changes: Player task admission

Layer / File(s) Summary
Folia admission arbitration
AdvancedCore/src/main/java/com/bencodez/advancedcore/api/rewards/RewardEligibility.java, AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/api/rewards/*
Folia scheduling uses FoliaLib’s scheduler. An atomic claim gates action execution, retirement, and rejection. Tests cover admission outcomes, callback ordering, and the non-Folia path.
Packaged FoliaLib API verification
AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/packaging/PackagedArtifactTest.java
The packaged-artifact test checks relocated FoliaLib class loading, scheduler and result types, enum constants, and absence of original FoliaLib classes.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 49d53

The admission ordering and null-result handling support the intended player-task behavior. No actionable issue remains before merge.

Security architecture risk: 🔵 Low · up to 49d53

The change strengthens protection against duplicate or late player-task callbacks while preserving player-identity checks. No introduced security concern was identified in the reviewed paths. Residual uncertainty concerns the external scheduler’s runtime behavior, which was not independently validated on a live server.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The material exposure is player reward and command execution within the hosting server. Callback arbitration can determine whether an effect executes or is retried, but the reviewed change does not grant additional identity or command authority. The claim is per submitted task, not a global exactly-once guarantee across crashes or independent submissions.

Trust Boundaries and Controls

  • inferred — A disconnect or retirement followed by a delayed callback is the relevant adverse sequence: work retained for retry must not also execute through the stale callback. The shared claim blocks that sequence at admission, while downstream identity checks and delivery claims provide additional controls before effects.

Resilience and Maintainability Implications

  • observed — Admission tests cover duplicate retirement, late dispatch, exceptional completion, unknown and null statuses, synchronous rejection, and relocated enum names. These are modeled regression checks for ownership and failure containment, not evidence of live-server execution or exhaustive concurrent scheduling behavior.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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 change: completing FoliaLib 0.5.1 player scheduler compatibility.
  • 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
  • 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

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 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-10-19T04:03:31.591723Z 49d5369 PR opened
🔒 Security Review ✅ Completed 2026-10-19T04:04:24.628534Z 49d5369 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.

@BenCodez
BenCodez merged commit c264734 into master Oct 19, 2026
6 checks passed
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