Skip to content

fix: re-check soft dependencies after every plugin has enabled - #78

Merged
JustinasLa merged 1 commit into
mainfrom
fix/late-soft-dependencies
Sep 29, 2026
Merged

JustinasLa merged 1 commit into
mainfrom
fix/late-soft-dependencies

Conversation

@JustinasLa

@JustinasLa JustinasLa commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On main (boot 2026-09-29 16:23), Paper broke the dependency cycle Cooking → CustomCrops → ItemsAdder → MMOItems → nightcore → MMOItems by enabling MMOItems after activity, despite the softdepend. Activity checked isPluginEnabled("MMOItems") during onEnable, found it off, and disabled every m.<type>.<id> path for the whole session:

  • daily rewards (m.loot.*_item_skin_scroll) → "Your rewards could not be handed over. Please make a ticket."
  • weekly m.* drops (Ignitium, skin scrolls) are affected the same way
  • m.* icons fall back to PAPER; the MMOItems station listener was never registered, so station crafting tasks weren't credited

Fix

  • Register hooks on the first server tick, after every plugin has enabled.
  • On that tick, reload the config when TLibs/MMOItems/MythicLib/ItemsAdder availability changed since enable.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved startup handling for item integrations. Changes in the availability of supported item plugins are now detected, and configuration is refreshed before integration hooks are registered. This helps item-related features use configuration that reflects which integrations are enabled when the server starts.

Paper breaks the Cooking/CustomCrops/ItemsAdder/MMOItems/nightcore
dependency cycle by enabling MMOItems after activity. Activity then
treated every m.<type>.<id> path as unusable for the whole session, so
daily/weekly reward scrolls failed with "could not be handed over",
m.* icons fell back to PAPER and MMOItems station crafts went uncredited.

Hooks are now registered on the first tick, and the config is reloaded
there when an item plugin's availability changed since enable.

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

coderabbitai Bot commented Sep 29, 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: a596160d-2108-402d-9d37-6c0965e243ef

📥 Commits

Reviewing files that changed from the base of the PR and between 7ef7fe1 and 871cee0.

📒 Files selected for processing (2)
  • src/main/java/net/tfminecraft/activitytf/ActivityPlugin.java
  • src/main/java/net/tfminecraft/activitytf/config/ActivityConfiguration.java

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


📝 Walkthrough

Walkthrough

Startup defers hook registration to a scheduled task. The task checks whether item plugin availability changed and reloads configuration before registering hooks when needed.

Changes

Plugin startup

Layer / File(s) Summary
Item plugin availability
src/main/java/net/tfminecraft/activitytf/config/ActivityConfiguration.java
load() uses a helper to identify disabled item-path plugins. itemPluginsChanged() compares cached plugin usability with current enabled states.
Deferred hook registration
src/main/java/net/tfminecraft/activitytf/ActivityPlugin.java
Startup schedules hook registration. If item plugin availability changed, the task logs a message and reloads configuration before registering hooks.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 871ce

Startup reconciles relevant plugin availability before registering hooks. A missing-plugin warning may become stale when overall item-path availability remains unchanged, but no material impact is established; no actionable merge risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 871ce

The change restores integrations that can become available late in startup without showing a new player-controlled route to rewards. A failure during the new startup reload could, however, leave some integrations inactive while the plugin remains enabled.

Retained concerns

  • Low · reliability · inferred: If the newly introduced startup reload fails, already registered core listeners can remain active with partially refreshed configuration while optional hooks are never registered. This can disrupt activity crediting and reward availability; no exception trigger or attacker control has been established.
Security review details

Security Blast Radius

  • inferred — The newly reachable behavior is the server's optional plugin listeners and the activity and reward configuration they consume. The supplied path does not establish a new player-controlled input to the startup decision.

Trust Boundaries and Controls

  • observed — Bukkit enabled-plugin state controls the new reload decision and still gates optional listener registration. Activity credit from the MMOItems listener continues through the manager's existing task checks.

Resilience and Maintainability Implications

  • inferred — An exception in the conditional reload would interrupt the same task before hook registration, although core listeners were registered earlier. The likelihood of such an exception and the server's task-failure behavior remain unverified.

Hardening Proposals

  • proposed — Define a failure policy for the deferred reconciliation, including whether to preserve the initialized configuration, retry, or disable affected integration behavior if reload cannot complete.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 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 summarizes the main change: re-checking soft dependencies after plugin startup to address startup-order issues.
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

A rabbit checks the plugin trail,
Then waits until the start is done.
If item plugins changed their state,
The config gets a fresh reload.
Hooks hop in when checks are through.

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

@JustinasLa
JustinasLa merged commit 0c6d158 into main Sep 29, 2026
2 checks passed
@JustinasLa
JustinasLa deleted the fix/late-soft-dependencies branch September 29, 2026 16:56
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