Repository navigation
feat(schedule): change the theme on a timer while the user is present - #22
Conversation
A restarted waybar or dunst was a child of whatever ran the apply, and setsid does not move it out of that process's cgroup. From a terminal it landed in the tmux pane's scope. From a systemd service, systemd kills the cgroup when the service exits, taking the new bar with it. start_session_daemon hands the program to Hyprland's exec dispatcher, so it runs in the login session's scope as it did at login. Without a Hyprland instance answering "ok" it falls back to the setsid launch. theme opacity restarts waybar through the same path instead of sending SIGUSR2. A SIGUSR2 reload is one of the triggers of the waybar 0.15 mpris use-after-free that reload_waybar already restarts to avoid.
apply_hyprland reloaded Hyprland itself, and reload_apps reloaded it again on the same apply. It was the only apply_* that reloaded its own app, so it now leaves the reload to reload_apps like the rest. reload_hyprland discarded only stderr, so hyprctl's "ok" landed between the per-app ticks of every apply.
calculate_usage_time merged every machine's applies into one timeline, so an apply on one machine ended the session another machine was still showing. A theme left on one machine for 103 days without a change was credited only until the next apply on any other machine. Each machine is now its own timeline. Another machine's latest apply is left out, since nothing here knows when its session ended, and only this machine's current theme runs to now.
theme schedule on installs a systemd user timer, or a launchd agent on macOS, that runs theme schedule run every five minutes. A run reads this machine's latest apply. A scheduled apply is due again after 6 hours and a hand apply after 12, so a theme picked by hand keeps the screen for half a day. A due change waits until the user is present: logind's IdleHint and LockedHint for the display session on Linux, ten minutes without input on macOS. Every change lands while the machine is in use. Apply rows carry a trigger, schedule or manual, and a row without one was applied by hand. theme rank adds SCHEDULED TURNS: a turn is kept when the machine's next apply was also scheduled, and switched away from otherwise, with the median time before the switch. Each run records its outcome in schedule.json, which theme schedule status reads: waiting, away, applied, blocked when the user manager has no display to reach, or failed when the apply wrote no history row. A scheduled apply pushes to the gist in the foreground, because the unit ends a backgrounded push when the run exits. The launchd agent sets AbandonProcessGroup for the same reason. WSL is refused, since it has no display session to read presence from.
datapointchris
left a comment
There was a problem hiding this comment.
PR review
Used: Ran bats history.bats schedule.bats opacity.bats apply-install.bats in the suite's sandbox, and all 65 passed. Read each test the diff adds or edits, and each existing test covering log_action, calculate_usage_time, set_waybar_opacity and apply_hyprland, beside the code it covers.
Standards: testing.md § "Test behavior the user cares about, not what this commit happened to change"; pull-requests.md § "Tests the diff adds are checked for whether they test the program or the change"; testing.md § "Never assert on rendered output — assert the value it was built from".
Findings: 0 correctness · 1 design · 0 prose
Design and architecture
-
tests/history.bats— the absent-key half of one log_action test repeats an assertion another test already makes. The affected test:tests/history.bats"log_action writes the message and trigger keys only when given", its first block (log_action apply gruvbox, then[has("message"), has("trigger")]==false,false).
Reach: "log_action writes one JSON object per call" runs the same
log_action apply gruvbox. It asserts the record's keys are exactlyaction,machine,platform,theme,ts. A blankmessageortriggerkey fails that assertion too. So the first block can only fail when its neighbor fails. The absent-trigger case also pins nothing a reader uses.apply_themealways passes a trigger, defaulting tomanual. The calls that leave it out arelike,dislike,note,rejectandunreject, and no reader looks at.triggeron those rows. Delete the first block and the: >"$THEME_HISTORY_FILE"reset after it. Keep the second block. It is the only test thatlog_actionwrites thetriggerthe schedule reads, sinceschedule.batsand theget_schedule_turnstest build their rows withadd_apply_record.
This review does not gate merge.
datapointchris
left a comment
There was a problem hiding this comment.
PR review
Used: I ran theme schedule status|on|off|run and theme rank from the branch. HOME and the XDG variables pointed at a scratch directory, and systemctl, loginctl, hyprctl and pgrep were stubs. I sourced the libraries with a stubbed apply_random to drive schedule_run through applied, failed (return and exit), away and waiting. I rendered SCHEDULED TURNS from a fixture history. I ran tests/schedule.bats, tests/history.bats and tests/opacity.bats, and all pass. I ran hyprctl version with HYPRLAND_INSTANCE_SIGNATURE unset.
Standards: help.md § "An error is the help screen for the failure in hand" · documentation.md § "Point at the source instead of copying it, and write the pointer with the source open" · testing.md § "Test behavior the user cares about, not what this commit happened to change". infrastructure.md § "Scheduled work reports its own outcome" is met, because schedule.json is read off the history row.
Findings: 1 correctness · 1 design · 1 prose
Correctness
lib/schedule.sh:158— The display guard checks the wrong variable. Every Hyprland call in a scheduled apply needsHYPRLAND_INSTANCE_SIGNATURE, and the guard never checks it. Without it,hyprctlanswersHYPRLAND_INSTANCE_SIGNATURE not set! (is hyprland running?)instead ofok.start_session_daemon(lib/lib.sh:2374) then takes thesetsidfallback. systemd kills that waybar when the oneshot exits, so the bar disappears.reload_hyprlandfails silently and the window colors stay.hyprctl hyprpaper wallpaperfails too, so the wallpaper stays. The history row is still written, soschedule.jsonrecordsappliedandstatusreports success.
Reach: a Hyprland session that exports onlyWAYLAND_DISPLAYto the user manager. The commonexec-once = dbus-update-activation-environment --systemd WAYLAND_DISPLAY XDG_CURRENT_DESKTOPline does exactly that. The help atbin/theme:494tells the user to supplyWAYLAND_DISPLAY or DISPLAYand names nothing else. This machine's manager carries the signature, which is why the body'ssystemd-runcheck passed.
The blocked message also says what is missing but not how to supply it. That breaks help.md § "An error is the help screen for the failure in hand".
Fix: on Arch, block unlesshyprctl versionsucceeds, and say so in the message. Name the import line in the blocked message and in--help:systemctl --user import-environment WAYLAND_DISPLAY HYPRLAND_INSTANCE_SIGNATURE. A cgroup-safe fallback instart_session_daemonwould cover non-Hyprland compositors as well:systemd-run --user --scopeinstead ofsetsid.
Design and architecture
tests/history.bats:35— The trigger half of "log_action writes the message and trigger keys only when given" pins the diff's shape. It does not pin behavior. Every reader treats an empty trigger and a missing one the same way.schedule_last_applyreads.trigger // "manual"and compares it to"schedule".get_schedule_turnsusesselect(.trigger == "schedule"). So no user can seehas("trigger"). Every caller that writes an apply passesmanualorschedule.
Reach: each later change tolog_action's record shape has to keep this assertion green. Nothing a user sees depends on it. testing.md § "Test behavior the user cares about, not what this commit happened to change".
Fix: delete the trigger assertions and keep themessagehalf. That half is real, becauseget_background_statsreads.message // empty. The hold and turn behavior thattriggerdrives is already pinned by the first test intests/schedule.batsand by theget_schedule_turnstest.
Prose
README.md:90andCLAUDE.md:213— "A theme the schedule applied keeps the screen for 6 hours, and one you applied by hand for 12" and "A scheduled apply is due again after 6 hours and a hand apply after 12." Both repeatTHEME_SCHEDULE_TURN_HOURSandTHEME_SCHEDULE_HOLD_HOURS.--helpalready builds its text from those constants. Changing either constant leaves these two sentences wrong, and nothing would notice. documentation.md § "Point at the source instead of copying it, and write the pointer with the source open".
Rewrite (README): "A theme the schedule applied keeps the screen for a shorter turn than one you applied by hand.theme schedule --helpgives both lengths."
Rewrite (CLAUDE.md): "A scheduled apply is due again afterTHEME_SCHEDULE_TURN_HOURSand a hand apply afterTHEME_SCHEDULE_HOLD_HOURS, both inlib/schedule.sh."
This review does not gate merge.
datapointchris
left a comment
There was a problem hiding this comment.
PR review
Used: ran bats tests/ at b12811d (252 pass), and ran theme schedule status, schedule run, schedule bogus and theme rank from the branch against a sandbox HOME with a seeded multi-machine history, with systemctl, hyprctl, pkill, tmux and gh stubbed. I also called schedule_decide and calculate_usage_time from a scratch script with uname stubbed to a prefixed hostname.
Standards: data.md § "Machine identity is a bare lowercased hostname"; testing.md § "Test behavior the user cares about, not what this commit happened to change"; documentation.md § "Point at the source instead of copying it, and write the pointer with the source open"
Findings: 2 correctness · 1 design · 1 prose
Correctness
-
lib/schedule.sh:60— On some machines the schedule never sees its own last apply, so it changes the theme at every 5-minute check while the user is active. With sync on, each of those runs also erases this machine's history rows. Reach: any machine whose hostname beginsmacos-,arch-,archlinux-,wsl-,linux-orunknown-, such aslinux-boxorarch-desktop. None of the four machines in the live history has one. The README offers the tool to anyone who installs it, though. The cause is inget_history. It runs every row throughnormalize_machine, which strips those prefixes as legacyplatform-hostlabels.--arg machine "$(_storage_get_machine_id)"is compared raw. Withunamestubbed tolinux-box, the written row says"machine":"linux-box"and reads back asbox.schedule_last_applythen returns nothing. One minute after a scheduled apply,schedule_decideanswersdue, wherestudioanswerswaiting. The same raw comparison has three more sites.calculate_usage_time(lib/storage.sh:183) returns{}for the current theme, so it never accrues hours._sync_own_records(lib/sync.sh:99) selects zero rows, so a push sends nothing._sync_merge_histories(lib/sync.sh:137and:145) keeps none of this machine's local rows. I merged a one-row local file with one remotemacminirow, and the result held only themacminirow. With sync on,schedule_runcallssync_pullbefore every apply, so each scheduled run deletes the rows from earlier runs. The sync sites predate this branch. The schedule is what turns them into a loss every 5 minutes.data.md§ "Machine identity is a bare lowercased hostname" says to canonicalize at read time, and that has to cover the value being compared too. Fix: givelib/storage.shone function that returns the id passed throughnormalize_machine, and use it at all five sites. -
lib/schedule.sh:383—theme schedule statussays a change is coming when the schedule is off. A user who has never runonseesStatus: Off. Below it they readNext change: due since Sat 20:25, at the first check while you are active. No check will ever run, so nothing will change. Reach: runningstatusbeforeon, which is the first thing the help's Examples suggest. I ran it against the sandbox history with no timer installed, and it printed exactly those two lines. Fix: print theNext change:line only when$onis true. When the schedule is off, theTurn it on with:line already says what to do.
Design and architecture
tests/history.bats:39— Thehas("trigger")half of "log_action writes the message and trigger keys only when given" checks a shape that no reader can tell apart. A row with"trigger":""reads as manual everywhere..trigger // "manual"yields"", andschedule_last_apply,schedule_statusandget_schedule_turnsall compare against"schedule". So a blank trigger and a missing one behave the same. The assertion fails only if someone changeslog_action's jq on purpose. Reach:testing.md§ "Test behavior the user cares about, not what this commit happened to change". Delete thehas("trigger")assertion. Keep the half that writesscheduleand reads it back, because nothing else pinslog_actionwriting the trigger.tests/schedule.batsseeds rows throughadd_apply_record.
Prose
CLAUDE.md:213andREADME.md:90— Both copy the values ofTHEME_SCHEDULE_TURN_HOURSandTHEME_SCHEDULE_HOLD_HOURS. CLAUDE.md says: "A scheduled apply is due again after 6 hours and a hand apply after 12." README.md says: "A theme the schedule applied keeps the screen for 6 hours, and one you applied by hand for 12."theme schedule --helpinterpolates the constants. These two sentences go stale the first time one of the constants is tuned. Rewrite in CLAUDE.md: "A scheduled apply is due again afterTHEME_SCHEDULE_TURN_HOURSand a hand apply after the longerTHEME_SCHEDULE_HOLD_HOURS, both inlib/schedule.sh." Rewrite in README.md: "A theme you applied by hand keeps the screen longer than one the schedule applied.theme schedule --helpgives both durations."
This review does not gate merge.
PR review — mergedThree reviewers ran: two general and one test reviewer. This comment unions reviews 5561116718, 5561123745 and 5561105245. The author's session launched the round through a review lead. Findings: 3 correctness · 1 design · 1 prose Correctness
Design and architecture
Prose
This review does not gate merge. 2 rule(s) proposed, filed into the fleet inbox as proposals. |
get_history passes every row's machine through normalize_machine, which strips legacy platform prefixes such as linux- and arch-. The id this machine compared against was the raw hostname, so on a host named linux-box no row ever matched. The schedule found no last apply and changed the theme at every check, the usage timeline never ran the current theme to now, and sync pushed nothing and dropped this machine's local rows on every pull. The id now goes through the same jq definition, so writes, comparisons and the gist filename agree with what reading produces.
…cannot take it A process the apply starts stays in the apply's cgroup even after setsid, and systemd kills that cgroup when a scheduled oneshot exits. Outside Hyprland the restarted waybar or dunst died with every scheduled change. systemd-run --user --scope moves it into a transient scope of its own, which outlives the service and still inherits the apply's environment. setsid stays for a machine with no user manager.
The display guard checked WAYLAND_DISPLAY and DISPLAY, but every Hyprland call in the apply also needs HYPRLAND_INSTANCE_SIGNATURE. A user manager without it still wrote the history row, so the check recorded applied while the window colors, the wallpaper and the restarted bar never changed. The guard now asks hyprctl whether it reaches a running Hyprland. A blocked check names each missing variable and the systemctl --user import-environment line that supplies it, and --help says the same.
With the timer off, status still printed when the next change was due, though no check would ever run. The Turn it on with line already says what to do.
README and CLAUDE.md restated the turn and hold hours, which --help already builds from the constants. Tuning either constant would have left both sentences wrong.
"log_action writes one JSON object per call" asserts the record's exact key set, so a blank message or trigger key fails it too. What stays is the only test that log_action writes the trigger the schedule reads.
The usage history cannot tell a theme someone chose to keep from one nobody looked at. Hours count every stretch between applies, idle ones included. This adds
theme schedule, a timer that changes the theme while the user is at the machine. A scheduled theme gets 6 hours, and a hand-applied one 12. Each apply now records whether the schedule or a person made it, sotheme rankcan count which scheduled turns were kept and which were switched away from.What to look at
schedule_runinlib/schedule.sh(b12811d, 441a66b). It runs the verdict, the desktop-environment guard, the pull, the backgrounded apply, then reads the outcome off the history row. Check that every path writesschedule.json, and that blocked and failed exit non-zero.start_session_daemoninlib/lib.sh(197c9c2, 1e4c50b). The literalokanswer is the only test that Hyprland's dispatcher took the command. Check that the fallback runs whenhyprctlis missing or answers anything else, and that it takessystemd-run --user --scopewhenever a user manager answers.apply_themeinbin/theme(b12811d) skipssync_after_actionfor thescheduletrigger. Check thatschedule_runpushes on the applied path and nowhere else._storage_get_machine_id(e1edd28),calculate_usage_time(3a4af1c) andget_schedule_turns(b12811d) inlib/storage.sh. The id now passes through the samenormalize_machinethatget_historyapplies to every row, and both readers group applies by machine. Check a machine with a single apply, and rows with nomachinekey.How it was verified
bats tests/passed on every commit, with 253 tests at the head.systemd-run --user --waitrantheme schedule runwithTHEME_ENV=development, whose empty history made a change due now. It applied a random theme to the live desktop, wrote"trigger":"schedule"into the history, and wroteappliedintoschedule.json. waybar and dunst survived the service's exit and sat in the login session's scope. Before 197c9c2 they sat in the scope of whichever terminal pane last ran an apply.setsid sleep, the way the old reload did, died when the service exited. One started throughsystemd-run --user --scopeoutlived it, in a scope of its own underapp.slice. So did a stand-in daemon thatstart_session_daemonstarted from a oneshot withHYPRLAND_INSTANCE_SIGNATUREunset, where the dispatcher could not take it.theme schedule runwithUnsetEnvironment=HYPRLAND_INSTANCE_SIGNATUREand a change due exited 1 and wrote no history row. It recordedblocked: no usable HYPRLAND_INSTANCE_SIGNATURE, so a change could not reach the desktop. From a terminal on the desktop, run: systemctl --user import-environment HYPRLAND_INSTANCE_SIGNATURE. With the manager's full environment, the guard found nothing missing.linux-box. Against the code before e1edd28, the new test counted 0 of the host's own rows, andschedule_decideanswereddueone minute after a scheduled apply. After it, the test counts 1 and the decision iswaiting.theme schedule onwrote both units and started the timer.systemctl --user start theme-schedule.serviceloggedwaiting: next change due Sun 09:46, 12 hours after the last hand apply.statusshowed the timer active and that check.offremoved both files, andsystemctl --user is-enabledanswerednot-found. With the schedule off,statusprintsStatus: Offand noNext change:line.SetIdleHint b true, status readaway (session 2 is idle). Afterb false, it readactive.onloaded the agent withAbandonProcessGroupset.launchctl kickstartran a check that recordedaway: … no input for 2d 12h.offunloaded the agent and removed the plist.What changes
theme schedule on|off|status|runis new. It is off untilon, and--helpsays how it decides.trigger.apply,randomandchangewritemanual, and the timer writesschedule. A row without the key is read as manual.theme applyends with the time of the next scheduled change.WAYLAND_DISPLAYorDISPLAY, andHYPRLAND_INSTANCE_SIGNATUREwhile Hyprland runs. The blocked message and--helpgive thesystemctl --user import-environmentline that supplies them.theme rank's hours now come from one timeline per machine. A theme left on one machine for a long stretch gains hours, and those hours still include idle time.theme rankadds SCHEDULED TURNS once any scheduled turn has finished.linux-,arch-,archlinux-,macos-,wsl-,unknown-) is recorded and compared without it, the way history was already read. Such a machine's gist file is named for the shorter id.theme opacityrestarts waybar instead of sending it SIGUSR2.okbetween the per-app ticks is gone.Decisions, and what they rejected
SetIdleHint, which--helpprints.statuswarns while IdleHint has not changed since login.KillMode=processon the service. It leaves the restarted bar in the finished service's cgroup, and systemd reports leftover processes on every run.normalize_machine. Writes, comparisons and the gist filename then agree with what reading produces. Rejected: normalizing at each comparison site, which leaves the next comparison to remember it.scheduletrigger, written after the run started, is what the apply produced.theme randomevery 6 hours. It writes the same row as a hand apply, so the history could not tell a kept turn from a switch, and it fires while nobody is at the machine.Risk and rollback
Nothing runs until
theme schedule on, andtheme schedule offremoves the units. A revert leavestriggerkeys in history rows, which no other reader uses. Runtheme schedule offbefore reverting, or the timer keeps callingtheme schedule run, which the reverted CLI does not have.What this does not do
ok, and the restart falls back to the systemd scope.The review
#22 (comment) — 3 correctness, 1 design, 1 prose.