test: cover Recycler and preserve pending refunds

[ryanbarlow97]

Recycler previously had no measured test coverage, and its pending-return handling could discard an older refund when a second escrow file arrived for the same player. This adds 45 tests with strict 100% line, branch and instruction gates across all production classes, and fixes three reproduced failures: colliding pending returns now retain and deliver both stacks; unreadable escrow/return files remain available for recovery; equal configured output velocity bounds no longer throw after recycling.

Tests cover configuration and legacy recipes, provider ordering and all six provider integrations, durability and deposit policy, GUI events, cancellation/confirmation, persisted escrow recovery, commands, lifecycle and scheduled effects. CI publishes HTML/XML coverage reports. Private invariant checks are simplified only where existing caller validation, Bukkit collection contracts, or fixed layout arithmetic already establish the condition. Resource null checks move before resource ownership to avoid unreachable compiler-generated cleanup branches while preserving close/error handling.

Validation: Java 21 mvn -o -B --no-transfer-progress clean verify: 45 tests, no failures/errors/skips; 1,316/1,316 lines, 728/728 branches, 5,310/5,310 instructions; no coverage exclusions. Each corrected inventory/effect failure was reproduced before its fix.

Remaining existing limitation: failed file deletion is not checked before a refund, so a readable return file that cannot be deleted could deliver twice. This needs a separate transactional persistence change. External plugin APIs are mocked; these results do not replace a live Minecraft integration run.

Summary by CodeRabbit

• Bug Fixes

  • Pending item returns now support multiple items per player and avoid overwriting existing pending files.
  • Items that are unreadable or empty remain available for later recovery instead of being discarded.
  • Recipe output handling and preview display have been adjusted for more consistent results.

• Documentation

  • Added guidance on running tests, coverage requirements, and where to find reports.

• Tests

  • Added automated checks for recycler workflows, item handling, recipes, and effects. Coverage reports are now published by CI.

[coderabbitai]

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: 60366e21-6ef3-4957-9424-c8777c1b8d48

📥 Commits

Reviewing files that changed from the base of the PR and between a93ce43 and e81420e.

📒 Files selected for processing (23)

• .github/workflows/build.yml
• .github/workflows/maven-release.yml
• README.md
• pom.xml
• src/main/java/net/tfminecraft/recycler/Messages.java
• src/main/java/net/tfminecraft/recycler/Recycler.java
• src/main/java/net/tfminecraft/recycler/loader/RecipeLoader.java
• src/main/java/net/tfminecraft/recycler/manager/EscrowManager.java
• src/main/java/net/tfminecraft/recycler/manager/InventoryManager.java
• src/main/java/net/tfminecraft/recycler/manager/RecyclerManager.java
• src/main/java/net/tfminecraft/recycler/util/GridLayout.java
• src/main/java/net/tfminecraft/recycler/util/ResultSpawnEffects.java
• src/main/java/net/tfminecraft/recycler/util/StationCompleteEffects.java
• src/test/java/net/tfminecraft/recycler/EffectsTest.java
• src/test/java/net/tfminecraft/recycler/EscrowManagerTest.java
• src/test/java/net/tfminecraft/recycler/InventoryManagerTest.java
• src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java
• src/test/java/net/tfminecraft/recycler/LoadersTest.java
• src/test/java/net/tfminecraft/recycler/ProvidersTest.java
• src/test/java/net/tfminecraft/recycler/RecyclerManagerTest.java
• src/test/java/net/tfminecraft/recycler/SocketProvidersTest.java
• src/test/java/net/tfminecraft/recycler/TestSupport.java
• src/test/java/net/tfminecraft/recycler/UtilitiesTest.java

💤 Files with no reviewable changes (1)

• src/main/java/net/tfminecraft/recycler/util/StationCompleteEffects.java

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

---

📝 Walkthrough

Walkthrough

The pull request configures Maven tests and JaCoCo coverage checks, adds test suites, updates resource, recipe, escrow, inventory, and effect code, and uploads coverage reports from build workflows.

Changes

Recycler tests and runtime

Layer / File(s)	Summary
Test and coverage setup pom.xml, src/test/java/net/tfminecraft/recycler/TestSupport.java	Adds JUnit, Mockito, and MockBukkit test dependencies, Surefire, JaCoCo report and coverage checks, and a shared test fixture.
Resource and recipe loading src/main/java/net/tfminecraft/recycler/Messages.java, src/main/java/net/tfminecraft/recycler/Recycler.java, src/main/java/net/tfminecraft/recycler/loader/RecipeLoader.java, src/test/java/net/tfminecraft/recycler/LoadersTest.java, src/test/java/net/tfminecraft/recycler/LifecycleCommandsTest.java	Resource-loading methods check for missing resources before opening streams. Recipe parsing changes how output sections, lists, and blank paths are handled. Tests cover loader, lifecycle, command, and resource error cases.
Escrow and recycler interactions src/main/java/net/tfminecraft/recycler/manager/EscrowManager.java, src/main/java/net/tfminecraft/recycler/manager/InventoryManager.java, src/main/java/net/tfminecraft/recycler/manager/RecyclerManager.java, src/main/java/net/tfminecraft/recycler/util/GridLayout.java, src/test/java/net/tfminecraft/recycler/EscrowManagerTest.java, src/test/java/net/tfminecraft/recycler/InventoryManagerTest.java, src/test/java/net/tfminecraft/recycler/RecyclerManagerTest.java	Escrow handling processes multiple pending files and avoids replacing an existing pending destination. Inventory, preview, and recycler manager logic also changes. Tests cover persistence, inventory displays, and recycler interactions.
Providers, utilities, and effects src/main/java/net/tfminecraft/recycler/util/*, src/test/java/net/tfminecraft/recycler/EffectsTest.java, src/test/java/net/tfminecraft/recycler/ProvidersTest.java, src/test/java/net/tfminecraft/recycler/SocketProvidersTest.java, src/test/java/net/tfminecraft/recycler/UtilitiesTest.java	Effect code changes velocity sampling and null-input handling. Tests cover effects, provider selection, socket providers, and utility behavior.
Coverage publication and instructions .github/workflows/build.yml, .github/workflows/maven-release.yml, README.md	Both workflows upload JaCoCo reports when the XML file exists. The README documents test execution, coverage requirements, report locations, and CI publication.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Merge Risk: 🔵 Low · up to e8142

Pending refunds now survive collisions and unreadable files. One disclosed edge case remains: if a pending file cannot be deleted, an item may be handed out twice. Merging is acceptable if the owner is aware of this and plans a transactional persistence follow-up.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to e8142

The refund fix improves recovery, but a partially completed file move can now accumulate multiple refunds across restarts. Downgrading can also leave newer refund files undiscoverable. These risks depend on recovery or rollback conditions rather than normal successful operation.

Retained concerns

• Medium · security · inferred: If fallback copy completes but source deletion fails or execution stops before deletion, the escrow source remains. Repeated enable/disable recovery now creates additional UUID-suffixed pending files for that same obligation, and delivery refunds every copy. This worsens recovery idempotency beyond the pre-existing single-file replay limitation.
• Low · reliability · inferred: Rolling back after suffixed refund files have been created makes those outstanding player obligations invisible to the base implementation's listing and delivery paths. The files remain on disk, but automatic ownership recovery requires reconciliation or restoration of the newer reader.

Security review details

Security Blast Radius

• inferred — The duplication outcome affects item ownership and the server economy. Every player whose readable escrow source survives a fallback copy can be affected; amplification grows with repeated recovery attempts before delivery.

Security Findings and Attack Paths

• inferred — The conditional path is failed rename, completed copy without source removal, repeated lifecycle recovery into fresh suffixed destinations, then delivery of every matching copy on the owner's join. Receiving the inflated refund requires no administrative command permission. Attacker ability to cause the prerequisite filesystem failure or lifecycle repetition is not established.

Trust Boundaries and Controls

• observed — Join delivery derives identity from the Player object and matches that UUID's filenames. Explicit administrative returns remain gated by recycler.admin. Inventory deposits retain GUI-holder, inventory and item validation at the caller despite removal of redundant private-handler checks.

Resilience and Maintainability Implications

• observed — Successful rename removes the escrow source and returns before fallback copying, preventing the identified accumulation path. Retaining unreadable files improves administrator recovery, but does not establish exactly-once handling of readable obligations after partial moves.

Hardening Proposals

• proposed — Use stable obligation identifiers and recoverable transition states so retrying an interrupted move cannot create a new refundable obligation. Detect incomplete cleanup and reconcile duplicate representations before delivery.
• proposed — Provide a downgrade reconciliation procedure for outstanding suffixed refunds, or require those obligations to be resolved before deploying the single-file reader.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 18 files. (4 skipped: …	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 changes: expanded Recycler test coverage and preservation of pending refunds during collisions and recovery scenarios.
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 2.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 18 files. (4 skipped: 4 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 tests at dawn,
Then hops through files from dusk till morn.
The coverage numbers fill the page,
While carrots wait beside the stage.
The build sends reports along,
And bunny hums a testing song.}

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