Skip to content

Give the Mise PATH cleanup a collision-free migration id - #12174

Open
ekollof wants to merge 1 commit into
omacom:quattrofrom
ekollof:fix/mise-migration-id-collision
Open

ekollof wants to merge 1 commit into
omacom:quattrofrom
ekollof:fix/mise-migration-id-collision

Conversation

@ekollof

@ekollof ekollof commented Sep 16, 2026 •

Copy link
Copy Markdown

Fixes #12173.

Renames the Mise Work PATH cleanup migration from 1789095456.sh to 1789584463.sh. The old id collides with the deleted Panther Lake kernel migration of the same name, so omarchy-migrate skips the Mise cleanup for anyone carrying a 1789095456 completion marker from that migration.

The rename makes the cleanup pending again. Two groups are affected, and a marker cannot tell them apart:

  • Accounts that only hold the PTL-era marker run the Mise cleanup for the first time.
  • Accounts that already ran it under 1789095456 run it again. A stock ~/Work/.mise.toml is left unchanged. A custom Work config the user trusted again after the first run is untrusted once more, which is what the migration already does when it finds a trusted custom config.

The only other reference update is the migration path in mise-work-path-test.sh. References to 1789095456 in 1789325478.sh (comment) and omarchy-kernel-migration-test.sh (old-world simulation) intentionally still name the historical PTL migration.

On base 45748a28, omarchy-kernel-migration-test.sh failed the superseded-migration assertion and passes with the rename, and mise-work-path-test.sh passes. On current quattro (b18ab495, head 5878a55f) omarchy-kernel-migration-test.sh fails both with and without this change because the pacman stub does not match the -- arguments added in #11495 (needs a ---aware fake pacman, e.g. #13365). mise-work-path-test.sh passes at head. The marker collision itself is unchanged on current quattro: the Mise cleanup is still shipped as migrations/1789095456.sh.

@llstrk

llstrk commented Sep 25, 2026

Copy link
Copy Markdown

Automated AI review

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Verified: the rename fixes the migration-marker collision described in #12173. At the base, an account holding a 1789095456.sh completion marker from the deleted Panther Lake (PTL) kernel migration never runs the Mise Work PATH cleanup from #11336. At this head, the renamed 1789584463.sh is pending for that account and removes the unsafe config. No correctness issue was found in the two-file change.

Synthetic account: stock ~/Work/.mise.toml, trusted, PTL-era 1789095456.sh marker
Real bin/omarchy-migrate from each tree, mise 2026.9.4

                        base 45748a28                 head 72c37a91
omarchy-migrate         nothing pending, no run       runs 1789584463.sh, marks it
~/Work/.mise.toml       kept, still trusted           removed
mise env in a ~/Work    exports <repo>/bin on PATH    recreated stock config is
repository                                            rejected as untrusted

Verified: the affected population is wider than PTL hardware. The PTL migration began with omarchy-pkg-present linux-ptl || exit 0, and omarchy-migrate records any zero exit as complete. So every account that ran migrations from a quattro tree between 2026-09-13 18:51Z and 2026-09-14 20:49Z holds the marker, on any machine. First-install provisioning from that window marks shipped migrations complete the same way (source reading). The PTL migration is not in any tag or release branch, so this concerns edge and development-checkout users.

Verified: the renamed file is byte-identical to the old one, 1789584463 appears nowhere else in the fetched branch and tag history, and the remaining 1789095456 references (the comment in migrations/1789325478.sh and the old-world simulation in omarchy-kernel-migration-test.sh) correctly name the historical PTL migration. On the PR's base, omarchy-kernel-migration-test.sh fails at the quoted "superseded migration" assertion and passes at head. mise-work-path-test.sh passes at base and head, and a copy of the head test pointed back at the old path fails at the first migration call, so the test does run the renamed file.

Re-running the cleanup is not a no-op for re-trusted custom Work configs

The description says the one-day overlap population "simply re-runs a no-op". Two groups are involved, and they behave differently:

  • Accounts holding only the PTL-era marker run the cleanup for the first time (the fix above).
  • Accounts that already ran the Mise cleanup under 1789095456 (on quattro since 2026-09-15) run it again under the new id. With a stock config, the re-run left the synthetic home unchanged apart from the new marker. With a custom ~/Work/.mise.toml that the user trusted again after the first run, as the migration's own message invites, the re-run revokes that trust again and prints the revocation message again.

Impact: for that second case, mise warns that ~/Work/.mise.toml is not trusted and applies none of its settings until the user runs mise trust again. The effect is conservative (trust is removed, never granted) and limited to edge and development accounts. A marker cannot distinguish a PTL-era run from a Mise-era run, so the code has no clean way to avoid it.

Suggested change: reword the description so maintainers accept this knowingly: PTL-marker holders run the cleanup for the first time, and accounts that already ran it get a re-run that is a no-op for stock configs but revokes a re-granted trust on a custom Work config once more. No code change is suggested.

Current quattro context: faffb8ee has since removed the kernel-test assertion quoted in the description, so on current quattro the rename no longer changes that test's result. The user-facing reason still applies, because current quattro still ships the Mise cleanup as migrations/1789095456.sh. Separately, omarchy-kernel-migration-test.sh fails on current quattro with or without this PR: #11495 made the omarchy-pkg-* helpers pass -- to pacman, while the test's pacman stub still reads $2 for -Q and its call-log check for the new kernel packages expects the old pacman -S command line. The "both suites pass" result therefore holds on the PR's original base, not after a rebase.


Review information

Test scope: Source review of base 45748a28, head 72c37a91 and current quattro d201fb95, including Git history and tags. Both repository tests ran in an isolated sandbox on base and head, and on current quattro with and without this change. Migration scenarios ran each tree's real bin/omarchy-migrate against synthetic home directories with mise 2026.9.4 and only the Mise cleanup migration installed. The PTL marker was created directly, not by running the deleted migration. Fresh-install marking and the size of the affected edge population were assessed from source and history only; packaging was not inspected.

AI process: Opus 5.5 Medium coordination and synthesis, independent Opus 5.5 Xhigh and GPT 6 Sol Xhigh technical assessments, Opus 5.5 Medium editorial check.

Opt out: To stop receiving these reviews, reply to this comment saying so.

@ekollof

ekollof commented Sep 26, 2026

Copy link
Copy Markdown
Author

Description updated. The re-run is not a no-op for every account:

  • A marker that only records the deleted PTL migration runs the Mise cleanup for the first time.
  • An account that already ran the cleanup under 1789095456 runs it again. A stock ~/Work/.mise.toml is unchanged. A custom Work config the user trusted again after the first run is untrusted once more. The marker cannot tell those two histories apart.

Also noted in the description: current quattro removed the superseded-migration assertion this branch was fixing in omarchy-kernel-migration-test.sh, and that test fails on current quattro with or without the rename because the pacman stub does not match the -- arguments from #11495. The marker collision is unchanged there: the Mise cleanup is still migrations/1789095456.sh.

@omarchybot omarchybot added the bug Something isn't working label Sep 27, 2026
@llstrk

llstrk commented Sep 28, 2026

Copy link
Copy Markdown

Automated AI review

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Follow-up to the earlier review and the author reply. The head is unchanged at 72c37a91, and current quattro is now 3faafba2.

Resolved: the updated description now covers both re-run groups. Accounts that hold only the PTL-era marker run the cleanup for the first time. For accounts that already ran it, re-runs on quattro with this PR merged showed that a stock config removed by the first run stays absent, and that a custom Work config re-trusted with mise trust (as the migration's message invites) is untrusted again. Trust granted through mise's trusted_config_paths setting or MISE_TRUSTED_CONFIG_PATHS survives the untrust under either id, while the migration still prints that trust was revoked. That is existing behaviour of the cleanup, not an effect of the rename.

Verified: the earlier verified results still hold on current quattro. The Mise cleanup still ships there as migrations/1789095456.sh, identical to this PR's renamed file. None of the migration runner, provisioning, the Mise and kernel tests or the omarchy-pkg-* helpers changed since the earlier review. No new migration uses either id. With the PTL-era marker, quattro leaves the migration un-run and the stock Work config in place. quattro with this PR runs 1789584463.sh and removes that config. The head merges cleanly into current quattro, and mise-work-path-test.sh passes on the merged tree. The context paragraph added to the description is accurate. Open #13365 and #13061 each fix the kernel test's pacman stub, and with #13365 that test passes with or without this rename.

Open #11322 reuses migrations/1789095456.sh for a different migration

Open #11322 adds its own migrations/1789095456.sh, a lock-screen fingerprint PAM repair. Today that is an add/add conflict with quattro, which still holds the Mise cleanup under that name. This PR moves the Mise cleanup away, so after it lands #11322 merges without a conflict. The fingerprint repair then carries an id already marked complete on the edge and development accounts listed below:

quattro + #12174 + #11322 (local merge), real bin/omarchy-migrate --pending
Synthetic accounts; the fingerprint migration itself was not run

account marker state             pending migrations
no 1789095456.sh marker          1789095456.sh (fingerprint repair)
1789095456.sh marker present     none (the repair is skipped)

The marker is held by three groups:

That is the #12173 collision again, for a third migration.

Impact: this PR is correct as it stands. But merging it removes the conflict that currently exposes the reuse, and #11322 has no discussion of it. If #11322 then merged with its current id, the fingerprint repair would silently not run for those accounts.

Suggested change: no change to this PR. #11322 needs a fresh migration id. This cross-reference is intended to make the dependency visible on both PRs.

Root cause (context): bin/omarchy-dev-add-migration names a new migration after HEAD's commit time (git log -1 --format=%cd --date=unix). The quattro merge commit of #11263 has committer time exactly 1789095456. Branches started from that commit (#11336, #11322, and the closed #11345 and #11346) all received that id. The PTL migration also carries the id, but how it got it was not established. A helper that uses the current time would avoid this, and a merge-time check against migration names already used in the target branch's history would catch the rest. That fits better in a separate issue than in this PR.


Review information

Test scope: Source and history review of head 72c37a91 and current quattro 3faafba2, plus open PRs that touch either migration id, the migration runner or these tests. Local merges of this PR with current quattro, #13365 and #11322 were built for testing. Both repository tests and the Mise cleanup scenarios ran in an isolated sandbox. The scenarios ran each tree's real bin/omarchy-migrate against synthetic homes, with mise 2026.9.4, including a trust-via-settings variant. All other migrations were pre-marked complete, and the PTL and provisioning markers were created directly. The #11322 effect was checked with --pending only. Its migration (sudo, /etc/pam.d) was not executed. Base tests were not re-run.

AI process: Opus 5.5 Medium coordination and synthesis, Opus 5.5 Xhigh technical review and final fact check, GPT 6 Sol Xhigh search for related issues, Opus 5.5 Medium editorial check.

Opt out: To stop receiving these reviews, reply to this comment saying so.

@ekollof
ekollof force-pushed the fix/mise-migration-id-collision branch from 72c37a9 to 5878a55 Compare September 28, 2026 18:33
@ekollof

ekollof commented Sep 28, 2026

Copy link
Copy Markdown
Author

Rebased onto current quattro (b18ab495); both migration suites pass on this tree. One note on direction, since faffb8ee removed the guard instead: the assertion was only the symptom. The underlying defect is the marker collision — omarchy-migrate tracks completion per filename under ~/.local/state/omarchy/migrations/, so anyone who completed the old PTL migration while it shipped already carries a 1789095456 marker and will silently skip the Mise PATH cleanup. Deleting the assertion leaves that population unreached with no signal. The rename makes the cleanup pending for everyone; its content is idempotent, so the overlap population simply re-runs a no-op. Happy to close this in favor of another approach if maintainers prefer one, but the stale-marker case needs covering somewhere.

@llstrk

llstrk commented Sep 29, 2026 •

Copy link
Copy Markdown

Automated AI review

Community review: Independent automated community review, unaffiliated with the Omarchy team, intended to help prepare PRs for their review.

Outcome: No code defects at 5878a55f, but the kernel test fails, contrary to the latest reply.

Kernel migration test still fails after the rebase

omarchy-kernel-migration-test.sh exits 1 at head and base: its fake pacman mishandles the -- from #11495. The description's "passes with the rename" fits only the old base 45748a28.

Suggested change: tie that sentence to 45748a28; optionally note the test needs a ---aware fake pacman (e.g. #13365).

Reproducer (fails at 5878a55)

Run in a checkout of the PR head 5878a55f (Arch/Omarchy, bash 5.3, GNU grep 3.12). The first two runs are the head as is; the sed only makes the test print the migration output and call log it otherwise deletes on exit. Keep </dev/null: with stdin left open, the unfixed fake pacman's grep waits on stdin instead of failing. The last run is the same head with #13365 merged (git fetch origin pull/13365/head && git merge --no-edit FETCH_HEAD). The base b18ab495 gives the same exit=1 as the head.

$ bash test/shell.d/omarchy-kernel-migration-test.sh </dev/null; echo "exit=$?"
exit=1

$ sed -i "s|^trap 'rm -rf \"\$scratch\"' EXIT|trap 'cat \"\$scratch/output\" \"\$CALL_LOG\"; rm -rf \"\$scratch\"' EXIT|" test/shell.d/omarchy-kernel-migration-test.sh
$ bash test/shell.d/omarchy-kernel-migration-test.sh </dev/null; echo "exit=$?"
Install the Omarchy kernel and make it the first Limine boot entry
Error: Package 'linux-omarchy' did not install
sudo pacman -S --noconfirm --needed -- linux-omarchy linux-omarchy-headers
pacman -S --noconfirm --needed -- linux-omarchy linux-omarchy-headers
exit=1

# Same head with #13365 merged (the fake pacman accepts --):
$ bash test/shell.d/omarchy-kernel-migration-test.sh </dev/null; echo "exit=$?"
ok - linux systems receive the generic Omarchy kernel and retain their recovery kernel
ok - linux-lts systems receive the generic Omarchy kernel and retain their recovery kernel
ok - linux-zen systems receive the generic Omarchy kernel and retain their recovery kernel
ok - linux-ptl systems receive the generic Omarchy kernel and retain their recovery kernel
ok - linux-omarchy-ptl-novrr-mm systems receive the generic Omarchy kernel and retain their recovery kernel
ok - linux-t2 systems are skipped even with other kernels installed
ok - a running T2 kernel is excluded even if its package is no longer installed
ok - ARM systems cannot receive an x86_64 kernel
ok - users who completed the PTL migration run the renamed migration with fresh completion markers
ok - new packages install, the central boot order is set, and the old kernel remains available
ok - repeat runs are a no-op after successful completion
ok - a partially installed kernel gets its missing headers
ok - existing new packages still receive the config repair and boot rebuild
ok - package failures leave the old boot setup intact and the migration pending
ok - a failed rebuild is retried even after config and package changes succeeded
ok - older Omarchy variants and fallback entries cannot satisfy generic kernel verification
ok - the exact generic kernel takes first position over previous PTL and custom orders
ok - the central boot order overrides legacy drop-ins and pacnew files without rewriting them
ok - a missing central config is created with the exact kernel first
exit=0

Earlier findings (previous review):

Reproducer: re-run revokes a re-trusted custom config (5878a55)

Run from the root of a checkout of the PR head 5878a55f. The account already holds a 1789095456.sh marker (the cleanup ran under the old id) and has a custom ~/Work/.mise.toml it trusted again afterwards. Only the renamed migration is installed in the temporary OMARCHY_PATH.

$ mise --version
2026.9.14 linux-x64 (2026-09-25)
$ cat repro.sh
home=$(mktemp -d); omarchy=$(mktemp -d)
mkdir -p "$omarchy/migrations" "$home/Work" "$home/.local/state/omarchy/migrations"
cp migrations/1789584463.sh "$omarchy/migrations/"
run() { env -i HOME="$home" XDG_CACHE_HOME="$home/.cache" XDG_CONFIG_HOME="$home/.config" \
  XDG_DATA_HOME="$home/.local/share" XDG_STATE_HOME="$home/.local/state" \
  OMARCHY_PATH="$omarchy" PATH="$PWD/bin:/usr/bin" "$@"; }
touch "$home/.local/state/omarchy/migrations/1789095456.sh"  # cleanup already ran under the old id
printf '[env]\nKEEP = "yes"\n' > "$home/Work/.mise.toml"      # custom Work config written afterwards
run mise trust "$home/Work/.mise.toml"                         # re-trusted, as the first run's message invites
echo "before: $(cd "$home/Work" && run mise env -s bash 2>&1 | grep KEEP)"
run bin/omarchy-migrate --pending
run bin/omarchy-migrate
echo "after: $(cd "$home/Work" && run mise env -s bash 2>&1 | grep -e KEEP -e 'not trusted')"
$ bash repro.sh; echo "exit=$?"
mise trusted ~/Work
before: export KEEP=yes
1789584463.sh

Running migration (1789584463)
Remove automatic project bin directories from PATH
mise untrusted ~/Work

Mise trust for this custom config was revoked. Review it before trusting it again:
  mise trust /tmp/tmp.2O2cshClA3/Work/.mise.toml
after: mise ERROR Config files in ~/Work/.mise.toml are not trusted.
exit=0
Verified
  • git range-diff shows the rebased commit equal to 72c37a91; the renamed migration is the same blob as migrations/1789095456.sh on quattro.
  • quattro after b18ab495 changes nothing relevant. No other branch, tag or open PR head uses 1789584463.
  • With a PTL-era marker, b421b1b4 skips the cleanup and keeps a trusted stock Work config. quattro plus this PR runs 1789584463.sh and removes it.
  • mise-work-path-test.sh passes at head. The head merges cleanly into b421b1b4.
  • With this PR merged, Recreate lock screen fingerprint PAM file for pre-quattro setups #11322 merges cleanly, and omarchy-migrate --pending omits its 1789095456.sh when that marker exists.

Review information

Test scope: Source and sandbox tests at 5878a55f, base, quattro b421b1b4 and local merges; #11322's migration not run.

AI process: Opus 5.5 Medium coordination and synthesis, Opus 5.5 Xhigh technical review and final fact check, GPT 6 Sol Xhigh search for related issues, Opus 5.5 Medium editorial check.

Opt out: To stop receiving these reviews, reply to this comment saying so.

@ekollof

ekollof commented Sep 29, 2026

Copy link
Copy Markdown
Author

Description updated: tied the passing claim to base 45748a2, noted kernel test fails at head/base due to pacman stub -- handling from #11495 (see #13365). mise-work-path passes at head. No code change.

@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

Reviewed at 5878a55f, reproduced on an Omarchy ISO worker, and found nothing to fix.

Reproduction. I set up an account that holds the PTL-era 1789095456.sh marker, with every other migration marked complete and a trusted ~/Work/.mise.toml that puts {{cwd}}/bin on PATH. I then ran each tree's real bin/omarchy-migrate against it. On quattro (8b4eae66), nothing is pending and the repository's bin stays on PATH. At this head, and with it merged into quattro, 1789584463.sh is pending, runs, and the repository's bin is gone from PATH. With the rename reverted, the bug comes back. Codex at medium effort reviewed the change separately and found no defects. It agreed that no other reference to 1789095456 needs to change, because provisioning discovers migration names dynamically and the remaining references name the historical PTL migration. Its review saw this review's notes, so its agreement isn't fully independent. The change is two lines, the size the problem needs.

Tests. mise-work-path-test.sh, migrate-scope, migrate-wrapper, migrate-notify, upgrade-to-quattro and config-test pass on the worker, on quattro and on the merge alike. omarchy-kernel-migration-test.sh fails on both, because of the pacman stub issue already noted (#13365). ./test/cli also fails identically on quattro and on the merge, at two assertions unrelated to this change. They are vscode generated theme references current theme file and vscode generated theme clears obsolete extension marker. Both expect behaviour that bin/omarchy-theme-set-vscode no longer has: a symlink, and a fixed extension version. With those two assertions made non-fatal, the remaining 114 checks pass on both trees.

Next. Waiting on the maintainer. The fix is verified, but the suite is not green on quattro itself until those two test/cli assertions are brought up to date.

@jandrusk

Copy link
Copy Markdown

Looks good — renames the Mise PATH cleanup migration off the deleted PTL-kernel id so omarchy-migrate actually runs it. Still needed on current quattro (1789095456.sh is still present; 1789584463 is not).

Happy to see this land; it unblocks #12173.

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

Development

Successfully merging this pull request may close these issues.

Migration 1789095456.sh collides with the deleted PTL-kernel migration of the same name

4 participants