[BenCodez]

Problem and ownership

MCHT finding 20261002T012032106596Z-aa5ef0416165 confirmed malformed normal Velocity reload replaces the active MQTT/admin configuration with defaults. SimpleAPI swallowed parsing failures; VotingPlugin also continued runtime reconfiguration when reload threw. This reproduces on current default-branch source; it is not a harness or demonstrated Bungee defect.

Fix

• Normal Velocity config reload uses the existing strict loader and only publishes successfully parsed input.
• Open input before Configurate so missing files cannot become an empty replacement snapshot.
• Reject failed soft/full reload before fencing votes, cancelling tasks, changing channels, or rebuilding transport/runtime.
• Keep the old snapshot/runtime and invalid disk contents. Log a bounded failure without dumping parser excerpts that may contain secrets.
• Preserve initial startup defaults and valid reload behavior. No cache migration, new setting, or simultaneous dependency/backend update required.

SimpleAPI library fix: BenCodez/SimpleAPI#104 . This PR works independently with the previously published SimpleAPI because the VotingPlugin adapter performs strict loading itself.

Validation

• Added regressions reproduced on unchanged source: 3 expected failures; strict Control preservation already passed.
• 34 focused config/Control/runtime-lifecycle tests passed.
• Full clean Maven package: 1,720 tests passed, including 4 packaged-artifact checks; 10 MiB gate passed.
• Full build passed with both the pre-existing dependency and the locally fixed SimpleAPI installation.
• Actual shaded VotingPlugin configuration class in an isolated JVM preserves MQTT/custom settings and unchanged malformed disk, then accepts repaired configuration.
• git diff --check passed; complete diff inspected; independent source-read-only review: No findings.

The previous hosted CI failed solely on the old hard size gate. #1687 is now merged, and this branch is rebased onto current master (93df54109). Updated head 0406ff7b109508061fa7f76ed2b9961fc161d089 passes a full local clean package: 1,813 unit tests + 5 artifact tests; oversized output produces the requested visible warning. Updated-head CI is pending.

MCHT host-side config_upgrade_regression remains pending execution access; the referenced CLI suite returns 404 through the retained machine API. An active dashboard suite has not been interrupted. No live transport takeover, vote loss, or live vote-safety claim is made.

Fixing candidate (both product fixes, independently rebuilt AdvancedCore dependency): SHA-256 3213eb6275b20816b97d154206e39f621cea8210fd72ff2089ccd06150f848d4. Both actual shaded configuration classes pass an isolated-JVM malformed/repaired reload probe. This is local candidate evidence, not the MCHT host regression result.

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
🔒 Security Review	✅ Completed 2026-10-02T01:55:20.198068Z	43d6ec7	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]

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: 87a4edb0-ac3d-4749-8827-68e9d12372e5

📥 Commits

Reviewing files that changed from the base of the PR and between 376c2a9 and 43d6ec7.

📒 Files selected for processing (4)

• VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VelocityConfig.java
• VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VotingPluginVelocity.java
• VotingPlugin/src/test/java/com/bencodez/votingplugin/proxy/velocity/VelocityConfigReloadTest.java
• docs/proxy-vote-delivery.md

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 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 (3)

VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VelocityConfig.java (1)

208-216: LGTM!

Also applies to: 220-224

VotingPlugin/src/test/java/com/bencodez/votingplugin/proxy/velocity/VelocityConfigReloadTest.java (1)

22-93: LGTM!

docs/proxy-vote-delivery.md (1)

56-63: LGTM!

---

📝 Walkthrough

Walkthrough

Velocity configuration reloads now propagate file and parsing failures. Runtime reload handling checks configuration first and returns on failure, preserving the active configuration and runtime. Tests cover malformed and missing files, startup defaults, and runtime state.

Changes

Velocity reload handling

Layer / File(s)	Summary
Strict configuration reload and validation VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VelocityConfig.java, VotingPlugin/src/test/java/com/bencodez/votingplugin/proxy/velocity/VelocityConfigReloadTest.java	VelocityConfig.reload() loads through loadControlConfiguration() and wraps load errors in UncheckedIOException. The control loader reads YAML from a buffered reader and propagates file and parsing errors. Tests cover malformed and missing reloads, repaired files, and startup defaults.
Guard runtime changes on reload failure VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/velocity/VotingPluginVelocity.java, VotingPlugin/src/test/java/com/bencodez/votingplugin/proxy/velocity/VelocityConfigReloadTest.java, docs/proxy-vote-delivery.md	reloadAllInternal returns when configuration reload fails, before fencing votes or canceling tasks. Tests assert that runtime state and tasks remain unchanged. Documentation describes the reload failure behavior.

Priority: ⬇️ Low

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant VotingPluginVelocity
  participant VelocityConfig
  participant bungeeconfig.yml
  participant ProxyRuntime
  participant Tasks
  VotingPluginVelocity->>VelocityConfig: reload()
  VelocityConfig->>bungeeconfig.yml: Open and parse YAML
  alt Configuration load fails
    VelocityConfig-->>VotingPluginVelocity: Throw reload failure
    VotingPluginVelocity->>VotingPluginVelocity: Log failure and return
    Note over VotingPluginVelocity,Tasks: Runtime state and tasks remain unchanged
  else Configuration load succeeds
    VelocityConfig-->>VotingPluginVelocity: Return loaded configuration
    VotingPluginVelocity->>Tasks: Cancel tasks during reload
    VotingPluginVelocity->>ProxyRuntime: Apply reload changes
  end

Loading

Merge Risk: ⚪ Minimal · up to 43d6e

Failed Velocity reloads retain the active settings and runtime. Malformed configuration at startup prevents startup rather than silently running with defaults; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 43d6e

The change improves failure containment by rejecting unreadable or malformed configuration before disrupting the active runtime. No introduced security issue was identified in the reviewed paths. Residual uncertainty concerns concurrent hosted administration and recovery behavior beyond configuration-load failure.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly reviewed failure-containment scope is the Velocity instance's active configuration, plugin-message channel, scheduled tasks, and proxy transport/runtime. Rejecting invalid input prevents that reload attempt from replacing settings or rebuilding those services; downstream fleet exposure was not exhaustively mapped.

Trust Boundaries and Controls

• observed — The normal Velocity command path requires votingpluginproxy.admin before dispatching reload or reloadall. Hosted Control uses a configured bearer credential and checks the negotiated configuration capability before applying routing changes. These caller controls are unchanged by the reviewed reload guard; server-side Control authorization policy was not inspected.

Resilience and Maintainability Implications

• observed — Preservation is specific to configuration-load failure, not a transaction spanning every full-reload stage. After successful loading, existing replacement paths can retire the old runtime and fail without restoring it. Those later paths are unchanged against the reviewed base and are not attributed to this PR.
• observed — The separate Control callback loads configuration and invokes reloadFromControl without acquiring Velocity's reloadLock. The same callback exists in the reviewed base. Cross-path concurrency is therefore a pre-existing coordination limitation, not a newly introduced callback or authority expansion; its safety is not directly demonstrated by the new tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 summarizes the main change: preserving the active Velocity configuration and runtime when a reload fails.

Full details: Docstring Coverage

Explanation

Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

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

Autopilot is currently an internal CodeRabbit preview.

---

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.}