Skip to content

Stop hibernation remove from leaving resume= on the UKI - #8199

Open
JustNak wants to merge 8 commits into
omacom:quattrofrom
JustNak:cursor/hibernation-remove-resume-dropins-1b5e
Open

JustNak wants to merge 8 commits into
omacom:quattrofrom
JustNak:cursor/hibernation-remove-resume-dropins-1b5e

Conversation

@JustNak

@JustNak JustNak commented Aug 25, 2026 •

Copy link
Copy Markdown

omarchy-hibernation-remove says it removes swap and boot resume settings. It deletes the swapfile and /etc/mkinitcpio.conf.d/omarchy_resume.conf, then runs limine-mkinitcpio. It never deletes /etc/limine-entry-tool.d/resume.conf. That drop-in is what setup writes for resume= and resume_offset=, and it is what limine-mkinitcpio bakes into the UKI. After remove, the swapfile is gone and the next UKI still tries to resume from it.

Setup will not repair this. It only writes resume.conf when the file is absent, so a leftover file keeps the old offset. Factory-reset already deletes resume.conf when the swapfile is gone for exactly that reason: a surviving offset drop-in "bakes the old swapfile's offset into the rebuilt UKI and breaks resume."

What changed

  • Remove deletes resume.conf before limine-mkinitcpio, so the rebuilt UKI has no resume= and no resume_offset=.
  • If the mkinitcpio hook is already gone but resume.conf remains, remove still deletes that file and rebuilds. It does not stop at "Hibernation is not set up".
  • /etc/limine-entry-tool.d/rtc-alarm.conf stays. Setup writes it only when the file is missing, and factory-reset leaves it. Remove does not treat that file alone as hibernation, and it does not rebuild.
  • /usr/lib/systemd/system-sleep/keyboard-backlight is left alone. That hook is useful for ordinary suspend.

The inverse, a hook present and the drop-in missing, is #7619.

Tests

$ bash -n bin/omarchy-hibernation-remove
$ bash -n test/shell.d/hibernation-remove-test.sh
$ ./test/shell.d/hibernation-remove-test.sh
ok - hibernation remove leaves the keyboard-backlight sleep hook
ok - remove deletes resume.conf before limine-mkinitcpio
ok - remove deletes leftover resume.conf even when the mkinitcpio hook is already gone
ok - remove is a no-op when hibernation is not set up
ok - remove is a no-op when only rtc-alarm.conf is present
ok - remove stops when resume.conf cannot be deleted
$ git diff --check

Fixes #10037

Fixes #10374

Fixes #12096

cursoragent and others added 2 commits August 25, 2026 10:20
Co-authored-by: JustNak <JustNak@users.noreply.github.com>
Co-authored-by: JustNak <JustNak@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 25, 2026 11:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends hibernation removal to clean stale Limine resume configuration safely.

Changes:

  • Removes resume and RTC alarm drop-ins before rebuilding initramfs.
  • Adds coverage for full, partial, and absent configurations.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
bin/omarchy-hibernation-remove Removes Limine resume drop-ins while preserving the suspend hook.
test/shell.d/hibernation-remove-test.sh Tests removal behavior and rebuild ordering.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/shell.d/hibernation-remove-test.sh Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@JustNak JustNak changed the title Cursor/hibernation remove resume dropins 1b5e Stop hibernation remove from leaving resume= on the UKI Aug 25, 2026
@omarchybot omarchybot added the bug Something isn't working label Sep 27, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed at 91bd9c3 against quattro (b18ab49). I found no defects.

The bug is real. On a worker built from the Omarchy ISO, I ran my own harness against bin/omarchy-hibernation-remove, with the /etc paths redirected onto a scratch tree and limine-mkinitcpio stubbed to record which drop-ins it would read. On quattro, a full remove still has resume=/dev/mapper/root resume_offset=… and rtc_cmos.use_acpi_alarm=1 in /etc/limine-entry-tool.d/ when the UKI is rebuilt. On this head, both drop-ins are gone before the rebuild, and unrelated drop-ins are untouched. Your test/shell.d/hibernation-remove-test.sh fails with the fix reverted ("remove deletes resume.conf") and passes 4/4 on the head. ./test/cli passes. I did not run the full ./test/shell.

The half-removed case matters most. Anyone who already ran omarchy hibernation remove on quattro is in exactly that state: the hook is gone and resume.conf is still there. On quattro, remove then says "Hibernation is not set up" and never rebuilds. On this head, it deletes the drop-ins and rebuilds. So this PR also repairs machines the old remove already left stale. The change to the script is proportionate to that problem.

Related: this fixes #10037, #10374 and #12096. All three report the same leftover resume.conf, and I have linked them to this PR. #12155 makes the same deletion inside a bundle of six unrelated fixes. It only covers the full-remove path, so a machine that already ran the old remove still gets "Hibernation is not set up" and keeps the stale parameters. This PR covers both paths, so I think it is the one to carry.

No second opinion. The configured reviewer (Codex Medium) did not run: its daily review budget was spent when this item came up. That is why this PR is not labelled verified yet. It now waits on a second review, and after that on the maintainer.

@omarchybot omarchybot added the verified Omarchy Triage has verified that this issue is ready for final review label Oct 1, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

The second opinion has now run. Codex Medium reviewed this head (91bd9c3) against quattro at c05d901. It agrees the bug is real. It also agrees this PR, not #12155, is the one to carry, because only this one repairs machines the old remove already left half-removed. Independence is not guaranteed, because the reviewer can read this session's notes. It raised one point I had not weighed, below.

Re-run on a worker. I merged this head with current quattro and ran the checks again on a fresh worker built from the Omarchy ISO. The harness from my earlier comment still fails on quattro, both for a full remove and for a half-removed machine, and passes on this head. Your test/shell.d/hibernation-remove-test.sh fails with the fix reverted and passes 4/4. unowned-system-paths-test.sh and system-sleep-ownership-migration-test.sh pass. ./test/cli fails one test, "vscode generated theme references current theme file". That test fails the same way on plain quattro, so it is not caused by this PR. On that evidence I've labelled this PR verified.

The open question is rtc-alarm.conf. Line 70 of bin/omarchy-hibernation-remove deletes /etc/limine-entry-tool.d/rtc-alarm.conf, and line 16 counts that file alone as "hibernation is set up". Codex's point is that rtc_cmos.use_acpi_alarm=1 controls RTC wake alarms in general, not only suspend-then-hibernate. Setup also keeps an existing file instead of writing its own, so remove can't be sure it owns the file it deletes. I checked the history: omarchy-hibernation-setup is the only thing in the repository that has ever written that file, and it always writes it together with the resume hook. So I don't count this as a defect. Whether removing hibernation should also drop the RTC flag is a behaviour choice. I have not changed it. If the answer is to keep the flag, the change is to delete only resume.conf and to check only resume.conf on line 16.

This waits on the maintainer for that call. It is not labelled ready, because this PR has no CI run and the second opinion's one point is still open.

`omarchy-hibernation-remove` deletes `/etc/limine-entry-tool.d/rtc-alarm.conf` when that file is the only marker. It then runs `limine-mkinitcpio`.

Setup writes that file only when it is missing. Factory-reset leaves it. A machine with only the RTC alarm is not hibernation.
The presence check treated `/etc/limine-entry-tool.d/rtc-alarm.conf` as hibernation. Setup writes that file only when it is missing, and factory-reset leaves it.

`omarchy-hibernation-remove` now enters for the resume hook or `resume.conf`. It deletes `resume.conf` before `limine-mkinitcpio`.
@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] Changes system hibernation removal script and boot configuration.

The PR appears safe to merge; the latest test change addresses the root-run failure without introducing a new actionable issue.

Summary

The PR removes the Limine resume drop-in before rebuilding the UKI and adds shell tests for configured, leftover, and absent hibernation state.

  • The latest change replaces a permissions-based deletion-failure test with a stub that also works when the suite runs as root.
  • No new actionable issue was identified.

Reviews (3) · Last reviewed commit: "test(hibernation): fail the resume.conf ..."

Comment thread bin/omarchy-hibernation-remove Outdated
@omarchybot omarchybot removed the verified Omarchy Triage has verified that this issue is ready for final review label Oct 2, 2026
`omarchy-hibernation-remove` still runs `limine-mkinitcpio` and prints "Hibernation removed" when `rm` cannot delete `resume.conf`.

The rebuilt UKI then keeps `resume=` and `resume_offset=`.
A failed `rm` of `/etc/limine-entry-tool.d/resume.conf` fell through to `limine-mkinitcpio`. The rebuild kept `resume=` and `resume_offset=`, and the script printed "Hibernation removed".

The command now exits before that rebuild.
Comment thread test/shell.d/hibernation-remove-test.sh Outdated
@omarchybot omarchybot added the verified Omarchy Triage has verified that this issue is ready for final review label Oct 2, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed at c6b0494 against quattro (821ae58). The two changes since my last comment hold up, and the open rtc-alarm.conf question is settled.

rtc-alarm.conf now stays. Remove deletes only resume.conf, and rtc-alarm.conf on its own no longer counts as hibernation being set up. That matches setup on quattro, which keeps an existing RTC file and never uses it to decide whether hibernation is configured. It also matches factory-reset, which deletes only the resume hook and resume.conf.

A failed deletion now stops the script. sudo rm -f "$RESUME_DROP_IN" || exit 1 on line 60 exits before limine-mkinitcpio and before "Hibernation removed". This answers Greptile's inline comment. Your new test makes the drop-in directory read-only and checks that remove fails without rebuilding. The test needs an unprivileged runner, because root can delete from the read-only directory. If root runs it, the test fails loudly rather than passing by accident.

Tests on a worker. I merged this head with current quattro and ran the checks on a fresh worker built from the Omarchy ISO. test/shell.d/hibernation-remove-test.sh passes 6/6. It fails on plain quattro, and it fails with the fix reverted. unowned-system-paths-test.sh and system-sleep-ownership-migration-test.sh pass. ./test/cli fails one test, "vscode generated theme references current theme file", and it fails the same way on plain quattro. I've labelled this head verified.

Second opinion. Codex Medium reviewed both pushes and found no defect in them. Independence is not guaranteed, because it can read my notes. It raised three points:

  • Base AGENTS.md prefers a full if/else to an exit in one branch. I didn't change anything for this. || exit 1 as a failure guard appears 108 times in bin/ on quattro, so it is the house idiom, not a departure from it.
  • The new test depends on not running as root. That is noted above.
  • A remove that stops at line 60 has already deleted the swapfile and the resume hook. This was true before this PR. Re-running remove now finishes the job, because a leftover resume.conf counts as hibernation still being set up.

This waits on the maintainer. It is not labelled ready, because ./test/cli is not fully green on a worker (for the unrelated reason above) and there is no CI run on this PR. This PR still fixes #10037, #10374 and #12096. It remains the better fix than #12155, which removes resume.conf only on the full-remove path and also deletes rtc-alarm.conf.

`chmod a-w` does not stop `rm` when the suite runs as root. The shell test then deletes `resume.conf` and fails its own assertion.

The failure case now stubs `rm` for that path.
@omarchybot omarchybot added verified Omarchy Triage has verified that this issue is ready for final review and removed verified Omarchy Triage has verified that this issue is ready for final review labels Oct 2, 2026
@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed at b1a66d2 against quattro (821ae58). The new commit fixes the root-run problem I noted last time, and I found nothing wrong with it.

The test now works as root too. The old version made the drop-in directory read-only, and root ignores that. The new version puts a stub rm on the test's PATH that refuses to delete limine-entry-tool.d/resume.conf and passes everything else to /usr/bin/rm. The stub applies only while remove runs, and the test deletes it before checking anything.

Tests on a worker. I merged this head with current quattro and ran the checks on a fresh worker built from the Omarchy ISO. test/shell.d/hibernation-remove-test.sh passes 6/6, both as an ordinary user and under sudo. With quattro's bin/omarchy-hibernation-remove in place of yours, it fails. unowned-system-paths-test.sh and system-sleep-ownership-migration-test.sh pass. ./test/cli fails one test, "vscode generated theme references current theme file", and I checked that it fails the same way on plain quattro. This head is labelled verified.

Second opinion. Codex Medium reviewed the new commit and the whole PR and found no defect. Independence is not guaranteed, because it can read my notes. It also confirmed that the failure path is reached whichever user runs the test.

This waits on the maintainer. It is not labelled ready, because ./test/cli is not fully green on a worker (for the unrelated reason above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working verified Omarchy Triage has verified that this issue is ready for final review

Projects

None yet

4 participants