v1.5.0: doctor overhaul, restart history, mod init fix, docs cleanup#12
Conversation
- Doctor command now reports live stats (TPS/memory/players), current restart status with remaining seconds and initiator, and the next scheduled restart time (previously only backend state, which made it look like restarts were not happening). - Added /reboot history command with restarts.log persistence. - Fabric/Forge/NeoForge now start health monitoring after the core engine enables, so monitoring no longer runs against an uninitialized backend registry. - Rewrote marketplace + README copy without emojis in plain language. - Fixed stale repository URLs (DemonZ-Development). - Bumped version to 1.5.0.
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
| if (core != null) { | ||
| core.onEnable(); | ||
| startPlatformMonitoring(); | ||
| } |
There was a problem hiding this comment.
Suggestion: core.onEnable() and startPlatformMonitoring() are invoked in the SERVER_STARTED callback without exception handling, so any runtime failure during backend/config initialization will propagate out of the lifecycle event and can abort or destabilize server startup. Wrap this block in a try/catch (as done in Forge/NeoForge) and log the full failure so startup continues gracefully. [possible bug]
Severity Level: Critical 🚨
- ❌ Fabric core enable can fail with uncaught exception.
- ⚠️ Monitoring startup on Fabric may destabilize server lifecycle.Steps of Reproduction ✅
1. Start a Fabric dedicated server with the RedstoneReboot Fabric mod enabled, which
invokes `onInitializeServer()` in
`fabric/src/main/java/dev/demonz/redstonereboot/fabric/RedstoneRebootFabricMod.java:38-70`.
2. Inside `onInitializeServer()`, the core engine is constructed via `startCore(...)` (see
`AbstractBootstrapServerPlatform.startCore()` at
`common/src/main/java/dev/demonz/redstonereboot/common/platform/AbstractBootstrapServerPlatform.java:52-59`),
and then a `SERVER_STARTED` lifecycle callback is registered at
`RedstoneRebootFabricMod.java:45-51`.
3. When the Fabric server finishes starting, the `ServerLifecycleEvents.SERVER_STARTED`
callback at `RedstoneRebootFabricMod.java:45-51` is executed, which sets `this.server` and
then, if `core != null`, calls `core.onEnable(); startPlatformMonitoring();` with no
surrounding try/catch.
4. If `core.onEnable()` (implemented at
`common/src/main/java/dev/demonz/redstonereboot/common/RedstoneRebootCore.java:60-81`,
which initializes backends, restart manager, and update checker, and calls
`platform.getTPS()` and `updateChecker.startPeriodicChecks(scheduler)`) or
`startPlatformMonitoring()` (implemented at
`AbstractBootstrapServerPlatform.java:166-179`, which constructs `PlatformLoadMonitor` and
starts monitoring) throws a runtime exception (e.g., due to backend initialization
failure, environment detection issues, or monitoring setup problems), that exception
propagates out of the Fabric `SERVER_STARTED` callback unhandled, potentially aborting or
destabilizing server startup for the Fabric platform, unlike Forge/NeoForge where the
analogous `onServerStarted` handlers at `forge/.../RedstoneRebootForgeMod.java:15-23` and
`neoforge/.../RedstoneRebootNeoForgeMod.java:15-23` wrap `core.onEnable()` and
`startPlatformMonitoring()` in a try/catch that logs errors and allows startup to
continue.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** fabric/src/main/java/dev/demonz/redstonereboot/fabric/RedstoneRebootFabricMod.java
**Line:** 47:50
**Comment:**
*Possible Bug: `core.onEnable()` and `startPlatformMonitoring()` are invoked in the `SERVER_STARTED` callback without exception handling, so any runtime failure during backend/config initialization will propagate out of the lifecycle event and can abort or destabilize server startup. Wrap this block in a try/catch (as done in Forge/NeoForge) and log the full failure so startup continues gracefully.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| } | ||
|
|
||
| public synchronized void record(String type, String reason, String initiator) { | ||
| ZonedDateTime now = ZonedDateTime.now(); |
There was a problem hiding this comment.
Suggestion: History timestamps are generated with the JVM default timezone instead of the configured server timezone used by scheduling/status output, which produces inconsistent times across commands and logs on hosts with different system timezones. Use the configured zone (or pass a time supplier from RestartManager) when creating event timestamps. [logic error]
Severity Level: Major ⚠️
⚠️ Restart history timestamps differ from status/doctor displays.
⚠️ Log file times depend on host default timezone.Steps of Reproduction ✅
1. Configure a server where the JVM default timezone differs from the RedstoneReboot
configured zone (e.g., JVM default UTC, plugin timezone Europe/Berlin). The configured
zone is provided by `PlatformConfig.getZoneId()` and used by `RestartManager` via `() ->
ZonedDateTime.now(config.getZoneId())` in
`common/src/main/java/dev/demonz/redstonereboot/common/manager/RestartManager.java` (lines
60-66).
2. Trigger scheduled or manual restarts so that `RestartManager` computes
`nextScheduledRestart` and schedules countdowns; `scheduleRestarts()` and
`calculateNextRestartTime()` in `RestartManager.java` (lines 105-132, 134-155) use
`currentTime()` (line 438) which delegates to the `nowSupplier` using the configured zone,
and `CommandProcessor.processStatus(...)` in
`common/src/main/java/dev/demonz/redstonereboot/common/command/CommandProcessor.java`
(lines 47-72) prints `rm.getNextScheduledRestart().format(STATUS_TIME_FORMAT) + " " +
core.getConfig().getTimezone()`.
3. When lifecycle events occur (schedule, immediate restart, execution result,
cancellation), `RestartManager` records them via `history.record(...)`: in
`scheduleRestart(...)` (lines 170-200, call at line 199), `performImmediateRestart(...)`
(lines 210-221, call at line 219), `handleExecutionResult(...)` (lines 316-351, calls at
324, 336, 342, 350), and `cancelRestart()` (lines 358-367, call at 366).
4. Each `record(...)` call reaches `RestartHistory.record(String type, String reason,
String initiator)` in
`common/src/main/java/dev/demonz/redstonereboot/common/manager/RestartHistory.java` (lines
33-48), which constructs `ZonedDateTime now = ZonedDateTime.now();` (line 34) using the
JVM default timezone. The resulting `Entry` is formatted by `Entry.format()` (lines 75-76)
and displayed in `/reboot history` via `CommandProcessor.processHistory(...)` (lines
165-175), and written to `restarts.log` via `Files.writeString` (lines 41-43). Because
history timestamps use the host default timezone while scheduler/status use the configured
zone, administrators see mismatched times between status/doctor output and history/log
entries when the two zones differ.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** common/src/main/java/dev/demonz/redstonereboot/common/manager/RestartHistory.java
**Line:** 34:34
**Comment:**
*Logic Error: History timestamps are generated with the JVM default timezone instead of the configured server timezone used by scheduling/status output, which produces inconsistent times across commands and logs on hosts with different system timezones. Use the configured zone (or pass a time supplier from `RestartManager`) when creating event timestamps.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
kindly fix these remaining issues
| } | ||
| controllerRestartPending.set(true); | ||
| logger.info("Restart accepted by Controller (" + backend.getName() + "). Local process ownership relinquished."); | ||
| history.record("EXECUTED", reason.getDisplayName(), restartInitiator); |
There was a problem hiding this comment.
Suggestion: The execution-result history entries use shared mutable state (restartInitiator) after the countdown state has already been reset, so executed/postponed/lockout events can be logged with the wrong initiator (often System). Capture the initiator at execution start and use that captured value when writing result history entries. [incorrect variable usage]
Severity Level: Critical 🚨
❌ Restart audit history misattributes executed restarts to "System".
⚠️ Doctor command shows wrong initiator during restart lifecycle.
⚠️ `restarts.log` unreliable for administrator troubleshooting.Steps of Reproduction ✅
1. On any platform, trigger a manual restart through commands. On Bukkit, `/reboot now`
invokes `handleNow(...)` in
`bukkit/src/main/java/dev/demonz/redstonereboot/bukkit/commands/RebootCommand.java` (lines
92-119), which calls `processor.processNow(new BukkitSender(sender), delay)` (line 117);
`/reboot schedule` similarly calls `processor.processSchedule(...)` (line 143).
2. In
`common/src/main/java/dev/demonz/redstonereboot/common/command/CommandProcessor.java`,
`processNow(...)` (lines 79-92) and `processSchedule(...)` (lines 94-107) obtain
`RestartManager rm = core.getRestartManager()` (lines 80 and 95) and call
`rm.scheduleRestart(delay, RestartReason.MANUAL/SCHEDULED_API, sender.getName())` (lines
86 and 101). `scheduleRestart(...)` in `RestartManager.java` (lines 170-208) assigns
`currentRestartReason = reason;` and `restartInitiator = initiator;` (lines 197-199), so
`restartInitiator` correctly holds the triggering user's name at this point.
3. When the countdown reaches zero, `startCountdown(...)` in `RestartManager.java` (lines
223-239) eventually calls `executeRestart()` (line 228). Inside `executeRestart()` (lines
250-313), the method saves `RestartReason reason = currentRestartReason;` (line 262) and
`RestartBackend backend = backendRegistry.getActiveBackend();` (line 263), then calls
`cancelCurrentCountdown(false);` (line 265). `cancelCurrentCountdown(boolean notify)`
(lines 370-381) cancels the task and resets fields: specifically it sets
`currentRestartReason = RestartReason.UNKNOWN;` and `restartInitiator = "System";` (lines
378-379), overwriting the original initiator.
4. After backend execution completes asynchronously, `executeRestart()` schedules
`handleExecutionResult(result, reason, backend)` with `scheduler.runLater(...)` (lines
283-289). `handleExecutionResult(...)` (lines 316-351) logs lifecycle events via history:
for accepted restarts it calls `history.record("EXECUTED", reason.getDisplayName(),
restartInitiator);` (lines 324 and 336), for failed restarts `history.record("POSTPONED",
...)` (line 342), and for unknown state `history.record("LOCKOUT", ...)` (line 350).
Because `restartInitiator` was reset to `"System"` in `cancelCurrentCountdown`, all these
history entries use `"System"` rather than the actual user name. `/reboot history` in
`CommandProcessor.processHistory(...)` (lines 165-175) and the `restarts.log` file
therefore show incorrect initiator information for executed/postponed/lockout events.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** common/src/main/java/dev/demonz/redstonereboot/common/manager/RestartManager.java
**Line:** 324:324
**Comment:**
*Incorrect Variable Usage: The execution-result history entries use shared mutable state (`restartInitiator`) after the countdown state has already been reset, so executed/postponed/lockout events can be logged with the wrong initiator (often `System`). Capture the initiator at execution start and use that captured value when writing result history entries.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix…for history, wrap Fabric startup in try/catch
…tters (RestartManager, RestartHistory, ServerEventListener, PlaceholderAPIHook, AlertManager, schedule calc)
User description
Summary
/reboot doctornow reports live stats (TPS, memory, player count), current restart status with remaining seconds and initiator, and the next scheduled restart. Previously it only showed backend state, which made it look like restarts weren't happening even when one was active./reboot historycommand: shows the last 10 restart lifecycle events (scheduled, executed, cancelled, postponed, lockout) with timestamp, reason, and initiator. Events are also appended torestarts.login the data folder.sdemonzdevelopment-specreferences updated toDemonZ-Development.Test plan
./gradlew :common:test— passes (includes restart-manager, command-processor, and integration tests; version assertion updated to 1.5.0)../gradlew :bukkit:compileJava— passes.commoncompiles and tests pass.Notes
Branch pushed from a fork (
Zentrix-Dev/RedstoneReboot) sinceZentrix-Devdoes not have push access toDemonZ-Development/RedstoneReboot.CodeAnt-AI Description
Add restart history and expand doctor output
What Changed
/reboot historyto show the last 10 restart events, including when they happened, why they happened, and who triggered themrestarts.logso they can be reviewed after a server restart/reboot doctornow shows live server stats, current restart progress, who started it, and the next scheduled restart timeImpact
✅ Clearer restart status during active countdowns✅ Easier review of recent restart events✅ Fewer startup issues on modded servers💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.