feat: add bedtime mode, and fix the hourly restart and dead scheduler - #24
Conversation
…hedules
Two bugs found while building bedtime mode. Both are pre-existing.
The update timer restarted the server every hour
systemd/minecraft-update.service ran `docker compose up -d --force-recreate`
unconditionally, on an hourly timer. --force-recreate recreates containers even
when nothing has changed, so everyone online was disconnected on the hour
whether or not a new image existed. The comment above the line says "Restart
container if image changed", but nothing checked.
It also restarted servers that had been stopped deliberately, which would have
undone bedtime within the hour.
scripts/auto-update.sh now pulls, compares the image id before and after, and
restarts only when it actually changed. A server that is not running is left
alone, because it was stopped for a reason and starting it again behind the
owner's back is worse than being a version behind. It also skips the pull in
that case, so a stopped server costs nothing hourly.
Scheduled commands were never executed
The Scheduler page writes to config/command-schedule.json, and
scripts/command-scheduler.py has a `run` subcommand that executes whatever is
due. Nothing invoked it: no timer, no cron entry, no loop in the API. Every
schedule created through the web UI was stored and silently ignored, and the
page gave no sign of it.
systemd/minecraft-scheduler.{service,timer} runs it once a minute, which is the
finest granularity the schedule format expresses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
W5 from the family server roadmap. A shutdown that arrives without warning starts an argument. Bedtime gives a countdown instead: a bossbar that empties, titles at the configured marks, and a goodnight message. The end of the evening becomes something the server announced rather than something a parent did. A window, not a moment Stopping the server is not enough on its own. A restart policy, an update timer or somebody pressing start on the dashboard all reopen the evening. So bedtime defines a closed window: between bedtime and the wake time, anyone who joins is sent straight back out with a message saying when the server opens again. That holds regardless of how the server came back. The window spans midnight, with anything before the wake time belonging to the previous evening, so 01:00 on Saturday is still Friday night. Weeknight and weekend bedtimes are chosen by the evening rather than the day, because it is the night before a non-school morning that matters. Controls Extend by a configured amount, skip tonight, or start bedtime now. Extending re-arms the warnings, so the countdown is announced again against the new deadline rather than going quiet. All three refuse once bedtime has happened and say why; a refusal is 409, because the request was well-formed and the server simply will not do it. Notes on two decisions Bedtime defaults to disabled. Something that can stop the server and turn people away should never switch itself on because a default said so. The join kick is queued rather than sent in the event handler. Bus handlers run on the thread that follows the server log, and sending a command makes a network call; this is the same trap that the Hall of Deaths review caught, avoided here rather than repeated. A bug the tests caught Warnings fired the largest unfired mark rather than the one describing the time actually left, so a server started with ten minutes to go greeted everyone with "30 minutes until bedtime". Every mark at or above the remaining time is now treated as given, and the smallest is the one announced. Elsewhere - New page at /bedtime with the countdown, the controls and the schedule. - GET /api/bedtime (server.view, so a read-only account can watch the countdown), and POST /api/bedtime/extend, /skip and /now (server.control). - config/bedtime.conf.example, and docs/BEDTIME.md linked from the docs index. All scheduling takes the current time as an argument, so a whole evening is tested without waiting for one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The tests used bats-assert's `--partial` flag and `refute_line`, neither of which exists in this repo's vendored helper. tests/helpers/bats-assert is a minimal stub: assert_output matches exactly, assert_line greps, and there are no refute helpers, so every partial match was being compared against the literal string "--partial". The script logic was correct throughout; only the assertions were wrong. They now use assert_line for substring matches, and negative checks are written as an explicit grep whose failure is asserted, wrapped in two named helpers so the intent stays readable. The stub docker also needed to answer `docker compose version`, which the compose() wrapper in scripts/lib/common.sh probes before running anything. Verified locally with bats this time rather than on CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
CI is green. Two notes on what happened on the way there. The bash test failure was mine, and the fix is worth knowing about. I wrote the tests using The Python failure on the first run was a pre-existing flaky test, not a regression.
No difference. The endpoint spends its time in a It passed on the re-run and I have left it alone, because raising the threshold again is whack-a-mole. The structural fix is that 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
There are a few correctness issues in the new code paths (notably concurrency/idempotence around bedtime enforcement and some UI request/handling details) that should be addressed before merging.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (4)
What changed in this PR
Adds “Bedtime mode” to enforce a warned, configurable evening shutdown window (with dashboard controls + REST/OpenAPI), and fixes two operational gaps: the hourly update timer restarting unconditionally, and the command scheduler never being invoked.
Changes:
- Implement Bedtime mode (API logic + endpoints + OpenAPI + dashboard page + docs/config) with a closed window that kicks late joiners until wake time.
- Fix hourly updates to restart only when the image actually changes, and keep deliberately-stopped servers stopped.
- Add a systemd timer/service to run scheduled commands once per minute (so schedules created in the UI actually execute).
| File | Description |
|---|---|
| web/src/services/api.js | Adds client methods for bedtime status + controls. |
| web/src/pages/Bedtime.jsx | New dashboard page for bedtime countdown, status, and controls. |
| web/src/pages/__tests__/Bedtime.test.jsx | Vitest coverage for Bedtime page behavior and control flows. |
| web/src/components/Layout.jsx | Adds Bedtime to the navigation. |
| web/src/App.jsx | Adds a lazy-loaded /bedtime protected route. |
| tests/unit/test-auto-update.sh | BATS unit tests for the new auto-update script behavior. |
| tests/api/test_bedtime.py | Pytest suite covering bedtime scheduling, window enforcement, controls, and endpoints. |
| systemd/minecraft-update.service | Switches hourly update unit to scripts/auto-update.sh run behavior. |
| systemd/minecraft-scheduler.timer | New timer to run scheduled commands every minute. |
| systemd/minecraft-scheduler.service | New oneshot service invoking scripts/command-scheduler.py run. |
| scripts/auto-update.sh | New script to pull/restart only on real image changes. |
| docs/INDEX.md | Links the new Bedtime documentation page. |
| docs/BEDTIME.md | Documents bedtime behavior, configuration, API, and operational notes. |
| config/bedtime.conf.example | Example config for enabling and tuning bedtime mode. |
| CHANGELOG.md | Documents bedtime feature + scheduler timer + update-timer fix. |
| api/server.py | Wires bedtime into the Flask API and event bus; adds endpoints and runner/stopper plumbing. |
| api/openapi.yaml | Adds Bedtime schemas and endpoints to the OpenAPI spec. |
| api/bedtime.py | New core Bedtime implementation: scheduling, countdown/bossbar, enforcement, and queued join kicks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
All four Copilot findings and both CodeQL alerts. Each was checked against the code; all six were real. Enforcement could close the evening twice _enforce() set the enforced flag without first checking it. tick() runs on the bedtime thread and start_now() runs on a request thread, and both check the flag before calling, so the check and the set sat in separate lock acquisitions. Two callers arriving together could both pass their check and both act: two goodnights, two kicks, two attempts to stop the server. _enforce() now does an atomic check-and-set under the lock and returns early if the evening is already closed. Worth noting about the tests: my first attempt spawned threads at a barrier and passed with the guard removed, so it proved nothing. The overlap is too narrow to reproduce reliably under the GIL. Calling _enforce() twice is the same condition without the timing, and that test does fail without the guard. The thread test is kept as a stress check and labelled as one. A server error was shown as a friendly refusal The dashboard treated any response carrying an `error` field as a refusal and showed it as a green notice. A 500 or 503 also carries that field, so a genuine server failure was dressed up as a normal "no extensions left" message, and a previous error could stay on screen underneath. Only 409 is a refusal now; everything else is an error, and the two clear each other. The page loaded twice on mount usePolling already calls its fetch function on mount, so the separate useEffect fired a second identical request every time the page opened. Bedtime controls left the cache stale Every other state-changing method in web/src/services/api.js calls invalidateCache(). The three bedtime POSTs did not, which matters most for "bedtime now", since it can stop the server while a cached /status still says it is running. CodeQL: both empty except clauses in the config parser now state why a bad value is ignored rather than raised. The three dashboard fixes have tests confirmed to fail without them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All four Copilot findings and both CodeQL alerts are fixed in aadb027. Each was checked against the code first; all six were real. The one that mattered is the enforcement race. My first test for it was worthless and I want to flag that rather than bury it. I spawned eight threads at a barrier, it passed, and it also passed with the guard removed. The overlap is too narrow to hit reliably under the GIL. Calling The refusal misclassification was also worse than it looks: the 500 and 503 paths on these endpoints return The other three: the page fired two identical requests on mount because All three dashboard fixes have tests confirmed to fail without them.
🤖 Generated with Claude Code |
Description
W5 from the family server roadmap, plus two
pre-existing bugs found while building it.
Bedtime mode
A shutdown that arrives without warning starts an argument. Bedtime gives a
countdown instead:
It is a window, not a moment. Stopping the server is not enough on its own,
because a restart policy, an update timer or somebody pressing start on the
dashboard all reopen the evening. Between bedtime and the wake time, anyone who
joins is sent straight back out with a message saying when the server opens
again. That holds regardless of how the server came back.
The window spans midnight, with anything before the wake time belonging to the
previous evening, so 01:00 on Saturday is still Friday night. Weeknight and
weekend bedtimes are chosen by the evening rather than the day, because it is
the night before a non-school morning that matters.
Controls are extend, skip tonight and start now. Extending re-arms the warnings
so the countdown is announced again against the new deadline. All three refuse
once bedtime has happened and say why.
New page at
/bedtime, endpoints under/api/bedtime, config inconfig/bedtime.conf.example, and docs/BEDTIME.md.Two bugs found along the way
Both are in the first commit, separate from the feature, and both are
pre-existing on
main.The hourly update timer restarted the server every hour.
systemd/minecraft-update.servicerandocker compose up -d --force-recreateunconditionally on an hourly timer.
--force-recreaterecreates containers evenwhen nothing has changed, so everyone online was disconnected on the hour
whether or not a new image existed. The comment above the line says "Restart
container if image changed", but nothing checked. It also restarted servers that
had been stopped deliberately, which would have undone bedtime within the hour.
scripts/auto-update.shnow compares image ids and restarts only on a realchange, and leaves a stopped server stopped.
Scheduled commands were never executed. The Scheduler page writes to
config/command-schedule.json, andscripts/command-scheduler.py runexecuteswhatever is due, but nothing invoked it: no timer, no cron entry, no loop in the
API. Every schedule created through the web UI was stored and silently ignored,
and the page gave no sign of it.
systemd/minecraft-scheduler.{service,timer}runs it once a minute.
Notes for review
The join kick is queued rather than sent in the event handler. Bus handlers
run on the thread that follows the server log, and sending a command makes a
network call. This is the same trap the Hall of Deaths review caught in #23,
avoided here rather than repeated.
Bedtime defaults to disabled. Something that can stop the server and turn
people away should never switch itself on because a default said so.
The tests caught a real bug. Warnings fired the largest unfired mark rather
than the one describing the time actually left, so a server started with ten
minutes to go greeted everyone with "30 minutes until bedtime". Every mark at or
above the remaining time is now treated as given, and the smallest is announced.
All scheduling takes the current time as an argument, so a whole evening is
tested without waiting for one, including the midnight boundary.
Type of Change
Checklist
Verification run locally:
make test-apimake test-webmake lintpasses (79 shell scripts through shellcheck, up from 77),docker compose configvalidates, Black and flake8 are clean, and theproduction web build succeeds.
Note: the two systemd units and the update-script change take effect only after
systemctl daemon-reloadand re-enabling on the Pi. The scheduler timer is newand needs
systemctl enable --now minecraft-scheduler.timer.Related Issues
🤖 Generated with Claude Code