Skip to content

Retire seamless login during Mac upgrade - #322

Open
CptPanko wants to merge 2 commits into
omacom:quattrofrom
CptPanko:fix/mac-upgrade-retire-seamless-login
Open

CptPanko wants to merge 2 commits into
omacom:quattrofrom
CptPanko:fix/mac-upgrade-retire-seamless-login

Conversation

@CptPanko

@CptPanko CptPanko commented Sep 2, 2026

Copy link
Copy Markdown

Summary

  • Hand off login ownership from seamless-login to exactly enabled SDDM during the Mac Quattro upgrade, with resumable interrupted states.
  • Preserve valid autologin and session selection through omarchy-mac-setup, and carry the fail-open Apple display guard to SDDM.
  • Remove legacy units, drop-ins, enablement links, and helpers only after the new login path validates.

Testing

  • env -u USER ./test/shell.d/mac-login-transition-test.sh (27 assertions)
  • ./test/shell.d/upgrade-to-quattro-mac-test.sh (12 assertions)
  • bash bin/omarchy commands --check (458 commands)
  • Full ./test/all in an ARM64 Ubuntu verifier: 2,683 passing assertions and the same exact 16 known failures as untouched quattro (2,656 passing); the same 22 test files fail out of 234 versus 233 on base.
  • bash -n for every touched shell file, git diff --check, conflict-marker, credential-pattern, and attribution scans.

Hardware validation

Real M1/M2 Asahi reboot, journal, VT, SDDM, and apple-drm validation was not available in this environment. The automated suite exercises condition polarity, setup-failure recovery, cleanup interruptions, and the fail-open display guard, but this PR does not claim real-hardware reboot validation.

Fixes #314

@scottjones

Copy link
Copy Markdown
Collaborator

Reviewed this alongside #319 and #321, since all three rewrite bin/omarchy-upgrade-to-quattro-mac. Two things — one specific to this PR, one about the three together.

Specific to this PR: login_cleanup_only infers more than it verifies.

if [[ ($seamless_state == "disabled" || $seamless_state == "not-found") ]] && legacy_artifacts_present; then
  require_sddm_alias
  login_cleanup_only=1
fi

Setting that flag then skips update_bashrc, replace_hyprland_config and run_quattro_setup in main(). So the script concludes "the config transition already completed" from the evidence "seamless-login is gone and some legacy artifact remains" — but those are different facts. A machine that never had seamless-login (not-found) and has one stale artifact lying around, say legacy_getty_dropin, takes this branch and skips the entire config phase while still printing "Upgrade complete."

legacy_artifacts_present returning true on any one of eleven paths makes that easier to hit than it looks. Worth either narrowing the evidence or recording explicitly that the config phase completed rather than inferring it — which leads to the second point.

The three PRs merge cleanly, and that is hiding a real conflict.

I trial-merged them: #319+#321, #319+#322 and #321+#322 all merge without conflict, and each merges cleanly into quattro. That's only because they append to disjoint regions. Semantically, merged, the script becomes ~1400 lines carrying two different resumability designs:

  • Make Mac config upgrades safely resumable #321 adds a durable state directory (~/.local/state/omarchy/upgrade-to-quattro-mac/), flock on fd 9 held for the script's lifetime, and six state/marker files with identity checks. It leaves main() alone.
  • This PR infers state from the live system, with no state directory and no lock, and is the only one of the three that changes main().

The concrete collision: after both land, main() is this PR's version, so replace_hyprland_config is #321's rewritten publish step — and in login_cleanup_only mode it gets skipped, while #321's backup_user_config has already taken the lock and written its markers. The upgrade exits reporting success with the config transition staged but never published. Neither test suite covers it, because neither could know about the other.

Note that #321's explicit state directory is exactly what would let this PR know the config phase completed instead of inferring it — the two problems have one answer.

What I'd like: rebase this and #321 onto each other and let them be reviewed as one unit, with a single answer to how this script tracks resumable state. Since both are yours you're best placed to reconcile them, and I'd take direction on which way you'd rather go.

Also worth acknowledging: the note in your description that real M1/M2 reboot validation wasn't available is the right call to make explicitly, and it's the kind of thing I'd rather see stated than glossed. I can run this on an M2 Max once the shape settles.

To be explicit about depth: this was an architecture-level review plus the login_cleanup_only path, not line-by-line across all +824. I'll do a full pass once the resumability question is settled.

@scottjones

Copy link
Copy Markdown
Collaborator

Following up on my earlier comment about reconciling this with #321 — separately, this PR needs eyes from the people who own the code it rewrites, and I'd rather say so than let it merge on my review alone.

@tayowrld wrote the SDDM-as-default path this restructures: setting up the sddm login as default for non-encrypted root and add migration to new sddm login for non-encrypted root users (Aug 23). Ivan, would you take a look? You have the most history in this area of anyone, and this PR retires the seamless-login handoff you built around.

The other half is mine. migrations/1784417043.sh, which this rewrites +9/-31, is login: wait for apple-drm before starting compositor (fix simpledrm blank boot) — that guard exists because of a real blank-boot failure on Apple Silicon, and this PR moves it into the new bin/omarchy-system-wait-for-display. Moving it is reasonable, but it is exactly the kind of guard that regresses silently: nothing fails until someone reboots into a black screen.

That matters more than usual here because the description is candid that real M1/M2 reboot validation was not available:

Real M1/M2 Asahi reboot, journal, VT, SDDM, and apple-drm validation was not available in this environment.

Stating that plainly was the right call. But it means the automated suite cannot cover the one thing most likely to break. I have an M2 Max and I wrote the original fix, so I'll take that piece: once the #321 reconciliation settles and the shape stops moving, I'll boot this branch and verify the apple-drm wait actually holds — VT handoff, SDDM start, and a cold boot that would have hit simpledrm.

Not asking for changes on this specific point. Just flagging that "tests pass" and "this boots" are different claims for this PR, and I'd like the second one established by hardware before it lands.

@malik-na

malik-na commented Sep 9, 2026

Copy link
Copy Markdown
Member

This still needs to be reconciled with #321 before line-by-line review. The two designs can report success with the config transition staged but unpublished; rebase them together and add a combined interruption test.

@malik-na

malik-na commented Sep 11, 2026

Copy link
Copy Markdown
Member

Needs work: reconcile this with #321. The combined cleanup-only flow reports success without publishing required config or reaching setup, despite passing existing suites. Use verified completion state and test main. Move retirement into a migration that reaches already-upgraded users, preserve administrator-authored files, and ensure the display helper exists before installing its service reference. Preserve effective autologin settings. Final review remains pending cold-boot/display-handoff validation.

@scottjones

Copy link
Copy Markdown
Collaborator

Re-reviewed against current quattro. Both blockers above hold up. Adding what I think is missing, starting with the one that changes what this PR is.

This fix does not reach the machine in #314. The reporter states it directly: "Reached quattro via bin/omarchy-upgrade-to-quattro-mac", and "Current state on this machine, post-upgrade" — both units enabled, greeter crash-looping. They have already crossed. omarchy-upgrade-to-quattro-mac only runs on a machine still making the 3 to 4 crossing, so putting the entire retirement there fixes future crossers and leaves every already-upgraded Mac in exactly the state the issue describes.

agents/skills/migrations.md names this case by hand:

Clearing a privileged file that a retired installer left on disk is the exception, and belongs in a migration whether or not that installer was part of a package layout transition. The upgrade command only runs on a machine still making the 3 to 4 crossing, so anything put there never reaches an install that crossed already [...] The upgrade command finishes by running omarchy-migrate (run_post_upgrade_migrations), so one migration reaches every population; a copy in the upgrade command would only be a second copy of the same predicate to keep correct.

/usr/local/bin/seamless-login is precisely that privileged file. Moving the retirement predicate into a migration covers both populations with one copy of the logic, and the crossing population still gets it because the upgrade ends by running migrations. The same paragraph also asks that such a migration "match what the old installer actually produced before deleting it", which bears on the next point.

The rewritten migration can install an ExecStartPre pointing at a file that may not be there. migrations/1784417043.sh used to write /usr/local/bin/omarchy-wait-for-display itself, so the drop-in it created could not reference a missing binary. It now writes ExecStartPre=/usr/bin/omarchy-system-wait-for-display and relies on something else having installed it. The comment says "installed by the Mac upgrade and by normal system-path wiring" — but on a checkout install /usr/bin/omarchy-* are regular files placed by a previous install pass, not live symlinks into the checkout, so a checkout that has the new command is not the same as a system that has it. On the machine I checked, all 472 of them are regular files.

If it is missing, ExecStartPre fails and omarchy-seamless-login.service does not start — a machine with no login path at all, which is the exact failure the guard was written to prevent. The old migration was fail-open by construction; this one isn't. A single line restores that:

[[ -x /usr/bin/omarchy-system-wait-for-display ]] || exit 0

The login_cleanup_only misfire has a more mundane trigger than a stale omarchy artifact. legacy_getty_dropin=/etc/systemd/system/getty@tty1.service.d/autologin.conf is a stock file — archinstall writes it, and plenty of people write it by hand for tty autologin. It is not evidence of seamless-login at all. A machine that never had seamless-login (not-found) and has that one file takes the cleanup-only branch, skips update_bashrc, replace_hyprland_config and run_quattro_setup, and prints "Upgrade complete."

The second half of that is worse than the skip: remove_legacy_login_artifacts then rm -fs the same file, plus plymouth-quit.service.d/wait-for-graphical.conf, without matching content — so an administrator's own autologin override is deleted on the way past. The migrations guide asks for the opposite: "Leave safe administrator-authored files alone."

Credit where it is due, and one thing that follows from it. validate_sddm_session_dir implements the layered precedence correctly — /usr/lib/sddm/sddm.conf.d, then /etc/sddm.conf.d, then /etc/sddm.conf last, which matches sddm.conf(5) exactly. That makes the autologin path's asymmetry read as an oversight rather than a misunderstanding: read_existing_autologin reads one file and ignores the layering it already knows how to walk. Note too that /etc/sddm.conf has the highest precedence, so an [Autologin] block there both wins at runtime and is invisible to this code — writing autologin.conf on such a machine is a no-op.

On severity for that blocker: the x86 omarchy-upgrade-to-quattro does the same thing today (reads only /etc/sddm.conf.d/autologin.conf, forwards only the user, hardcodes Session=omarchy.desktop). Worth fixing here, but it is parity with upstream rather than a Mac-specific regression, so I would not hold the PR on it alone.

One ordering soft spot. I traced the sequence and it is sound: sddm is in install/omarchy-base.packages so command -v sddm succeeds inside configure_autologin, install_session_file finds omarchy.desktop via the OMARCHY_PATH the upgrade passes explicitly, and install/config/enable-services.sh enables sddm during run_quattro_setup, before cleanup_login_transition calls require_enabled_sddm. The soft spot is the failure case: if sddm lands in install_quattro_packages' skipped list — which is warn-only — configure_autologin returns silently and the upgrade dies several steps later at "mac-setup did not produce valid SDDM autologin". That is a confusing message for "sddm failed to install."

Still unreconciled with #321. Re-verified today: both branches still merge cleanly into quattro and cleanly with each other, and merged the script is 1135 lines with two different resumability designs. main() is this PR's, so #321's staged replace_hyprland_config is skipped whenever login_cleanup_only=1 while its lock and markers have already been written. Both suites pass green on the merged tree. Neither branch has moved since the reconciliation request.

Hardware validation. My offer from 2026-09-05 stands and I intend to honour it. The apple-drm wait moves from seamless-login's ExecStartPre to SDDM's, and nothing about that fails until someone cold-boots into simpledrm. I wrote the original guard and I have the M2 Max, so once the shape stops moving I'll boot this branch and verify the VT handoff, SDDM start, and a cold boot that would have hit simpledrm. The automated suite genuinely cannot cover that, and the PR description is right to say so.

Verified locally: focused suite passes (27 and 12 assertions), bin/omarchy commands --check 455 commands, ./test/cli green, config-test.sh shows no new PKGBUILD coverage gap for bin/omarchy-system-wait-for-display. The branch is 194 commits behind quattro.

@wesleygrimes wesleygrimes added the defer Parked. Needs a decision, or may die with the fork. Do not close or port yet. label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

defer Parked. Needs a decision, or may die with the fork. Do not close or port yet.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mac quattro upgrade never retires omarchy-seamless-login, so SDDM's greeter crash-loops on every boot

4 participants