Commit e5d3556
refactor(menubar): consolidate icon-restore paths via LayoutReconciler (#604)
* refactor(menubar): extract icon-restore decision logic into pure planners with tests
Phase 1 of the icon-restore-paths consolidation effort. The four restore paths in `MenuBarItemManager` (`applyProfileLayout`, `restoreItemsToSavedSections`, `relocateNewLeftmostItems`, `relocatePendingItems`) have co-evolved through years of bug fixes and currently share no testable substrate, which makes any larger consolidation faith-based. This change introduces a regression-locking test net before the larger refactor begins so the next phase (true unification or reconciler model) can keep the bug-fix history green.
Adds six pure planning functions as `nonisolated static` methods on `MenuBarItemManager`: `planRebalanceMove`, `planLeftmostMove`, `planPendingMove`, `planNotchOverflow`, `planLCSMoveSequence`, `planFullSortSequence`. Each takes its inputs at the call boundary (pre-computed `sectionByWindowID` maps, live bounds, widths) and returns a typed decision; no `Bridging` or `NSScreen` access inside the planners. The orchestrators now derive the live-state inputs (Window Server section classification, notch geometry, hidden divider bounds) and delegate the decision step to the planner, but keep ownership of execution (`move()`), persistence (`pendingRelocations`, `knownItemIdentifiers`, `savedSectionOrder`), cooldown / settling, and cursor management.
Supporting changes: `ControlItemPair` promoted from `private` nested to internal with a new memberwise initializer so tests can construct fixtures; `MoveDestination` declared `Equatable` so planner results can be asserted directly; `longestCommonSubsequence` made `nonisolated private static`. Public signatures of all four orchestrators and call sites in `ProfileManager.apply` and `cacheItemsRegardless` are unchanged.
Adds 49 characterization tests across seven new files in `ThawTests`: `MenuBarTestFixtures.swift` (with sanity tests), `PlanRebalanceMoveTests.swift` (7 scenarios including multi-instance fungibility and `alwaysHidden`-disabled merge), `PlanLeftmostMoveTests.swift` (8 scenarios including Thaw-icon recovery and identifier migration), `PlanPendingMoveTests.swift` (8 scenarios including the `waitForRelaunch` sentinel and stored-neighbor precedence), `PlanNotchOverflowTests.swift` (8 scenarios with an explicit regression lock for the May 13 double-counted-`NSStatusItemSpacing` fix), `PlanLCSMoveSequenceTests.swift` (7 scenarios including the already-moved-becomes-stable-anchor chain), and `PlanFullSortSequenceTests.swift` (4 scenarios). Test fixtures use windowIDs in the 100s for non-classifying tests; the classifying tests bypass the live Window Server entirely by passing a pre-computed `sectionByWindowID` map to the planners.
Full `ThawTests` suite: 587 pass, 0 fail.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): position-aware icon restore via extracted planners
The four icon-restore paths in `MenuBarItemManager` (`applyProfileLayout`, `restoreItemsToSavedSections`, `relocateNewLeftmostItems`, `relocatePendingItems`) had co-evolved through years of bug fixes and shared no testable substrate. With profiles shipping in a major release, this change consolidates the decision logic behind a set of pure planner functions, adds a regression-locking test net, and fixes the long-standing position loss that occurred whenever an app was quit.
The first step extracts the decision logic of each orchestrator into `nonisolated static` planners with no `Bridging` or `NSScreen` access: `planRebalanceMove`, `planLeftmostMove`, `planPendingMove`, `planNotchOverflow`, `planLCSMoveSequence`, and `planFullSortSequence`. The orchestrators pre-compute live state (section snapshots, hidden divider bounds, item widths) and delegate the decision step to a planner; execution (`move()`), persistence, cooldowns, and cursor management remain in the orchestrator. `ControlItemPair` is promoted from `private` nested to internal with a memberwise initializer so `MenuBarTestFixtures` can construct synthetic instances; `MoveDestination` gains an `Equatable` conformance; `longestCommonSubsequence` becomes `nonisolated private static`. Public orchestrator signatures and the four call sites in `ProfileManager.apply` and `cacheItemsRegardless` are unchanged.
The second step introduces position awareness for the non-profile restore path. `savedSectionOrder` is now treated as an ordered desired layout per section, not just a per-baseID count. Two new planners are added: `planWithinSectionReorder` performs an LCS-based within-section reorder (with multi-instance-baseID fungibility) and runs after `planRebalanceMove` returns nil, emitting one move per call under the same 5s cooldown contract; `planUnmanagedPlacement` decides where each unmanaged item should land during a profile apply via `.saved(section,index)` / `.newItemDefault(section)` / `.newItemAnchored(section,anchor,relation)`. The hardcoded "park all unmanaged at visible-leftmost" in `applyProfileLayout` is replaced by per-uid placement that respects the user's prior layout. `savedPosition(for:in:)` and `savedPositionByBaseID(for:in:)` provide the exact-then-baseID lookup both planners consume.
The third step makes the cross-section restore destination itself position-aware and unifies the three position-aware paths on a single helper. `anchorDestination(forSavedIndex:inSection:savedSequence:currentUIDsInSection:)` is the shared "given a saved (section, index), produce an anchor-based destination" primitive; forward-first successor scan, backward predecessor fallback, section boundary as last resort. `RebalanceMove` gains a `destination: LCSPlannedDestination` field that the orchestrator resolves the same way `planLCSMoveSequence` already did; the previous hardcoded section-boundary destination is replaced. `planWithinSectionReorder` and `applyProfileLayout`'s unmanaged-item insertion are refactored to use the shared helper, eliminating two separate inline anchor scans. This fix is what stops a relaunched app from displacing the Thaw chevron when its saved position is non-leftmost.
The fourth step fixes a pre-existing bug in `saveSectionOrder(from:)` that the now position-aware restore exposed: when an app quit, its `savedSectionOrder` entry was appended to the end of the new identifiers list, destroying the user's positional intent on every quit. The new `planSectionOrder` helper rebuilds each section by starting with the currently-cached items in cache order and splicing closed-app entries in at positions anchored against their old-saved neighbors (same forward-first / backward-fallback rule as `anchorDestination`). Stale instance indices and cross-section moves are still pruned correctly.
User-visible effect after this change: a previously-seen app that relaunches into the always-hidden region is restored to its actual saved position rather than to leftmost-visible. The chevron stays leftmost. Profile applies preserve saved positions for unmanaged items instead of forcing them to visible-leftmost. Manual drags persist and are honored across quits because closed-app positions are no longer destroyed.
Adds a `ThawTests/MenuBarTestFixtures.swift` module with `MenuBarItem.fixture(...)`, `ControlItemPair.fixture(...)`, and `MenuBarItemTag.appItem(...)` plus 12 new test files covering every planner: `MenuBarTestFixturesTests`, `PlanRebalanceMoveTests` (including the position-aware destination assertions), `PlanLeftmostMoveTests`, `PlanPendingMoveTests`, `PlanNotchOverflowTests` (with the double-counted-`NSStatusItemSpacing` regression lock), `PlanLCSMoveSequenceTests`, `PlanFullSortSequenceTests`, `SavedPositionLookupTests`, `PlanWithinSectionReorderTests`, `PlanUnmanagedPlacementTests`, `AnchorDestinationTests`, and `PlanSectionOrderTests`. Fixtures use window IDs in the 1_000_000+ range and bypass the live Window Server entirely by passing pre-computed `sectionByWindowID` maps to the planners; the live AX / settling / CC-ownership state is never touched by a test.
Full `ThawTests` suite: 626 pass, 0 fail.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): split layout planners into LayoutSolver and PendingLedger
The previous commit moved the four icon-restore orchestrators behind a set of pure planner functions but kept them grouped as an extension on `MenuBarItemManager`. The structure mixed snapshot-pure decision logic (cross-section rebalance, within-section reorder, notch overflow, LCS sequencing, full-sort sequencing, unmanaged placement, saved-section rebuild, anchor resolution, saved-position lookup) with one genuinely history-dependent planner (per-entry pending-relocation decision that consumes the waitForRelaunch sentinel and `pendingReturnDestinations` ledger state). The LLM council's "Path C" recommendation called for separating these two by temporality so future contributors do not need to re-discover which planners depend on retry state. This change physically realizes that split.
Adds `Thaw/MenuBar/MenuBarItems/LayoutSolver.swift` as a static-only `enum` containing the snapshot-pure planners and their result types: `planRebalanceMove`, `planLeftmostMove`, `planNotchOverflow`, `planLCSMoveSequence`, `planFullSortSequence`, `planWithinSectionReorder`, `planUnmanagedPlacement`, `planSectionOrder`, `anchorDestination`, `savedPosition`, `savedPositionByBaseID`, plus the `longestCommonSubsequence` helper. Result types `RebalanceMove`, `LeftmostMove`, `NotchOverflowResult`, `LCSPlannedMove`, `LCSPlannedDestination`, `WithinSectionMove`, `UnmanagedPlacement`, and `SavedPosition` move with them. Every function is `nonisolated static` and pure over its inputs; nothing inside the file calls `Bridging` or `NSScreen`.
Adds `Thaw/MenuBar/MenuBarItems/PendingLedger.swift` as the matching namespace for the history-dependent decision logic: `planPendingMove` along with `PendingEntry` and `PendingMove`. The orchestrator (`relocatePendingItems`) still owns the `pendingRelocations` and `pendingReturnDestinations` mutable state and the raw-string sentinel persistence; the ledger file owns the typed shapes and the per-entry decision algorithm.
Updates the four orchestrator call sites in `MenuBarItemManager.swift` to invoke `LayoutSolver.planXxx(...)` / `PendingLedger.planPendingMove(...)` instead of `Self.planXxx(...)`. Updates the 12 planner test files (`PlanRebalanceMoveTests`, `PlanLeftmostMoveTests`, `PlanPendingMoveTests`, `PlanNotchOverflowTests`, `PlanLCSMoveSequenceTests`, `PlanFullSortSequenceTests`, `PlanWithinSectionReorderTests`, `PlanUnmanagedPlacementTests`, `PlanSectionOrderTests`, `AnchorDestinationTests`, `SavedPositionLookupTests`, plus the existing `MenuBarTestFixtures*Tests`) to reference the new fully-qualified names. The four orchestrator functions remain in `MenuBarItemManager.swift` per the council's warning against collapsing them into one method.
`MenuBarItemManager.swift` shrinks from 7,770 lines to 6,567 lines (net `-1,203`). `LayoutSolver.swift` is 1,099 lines; `PendingLedger.swift` is 178 lines. Doc comments at the top of each new file explain the temporality boundary so future contributors understand which planner belongs where.
Pure refactor: no behavior change, no new tests, no removed tests. Full `ThawTests` suite remains at 626 pass, 0 fail. Public API of `MenuBarItemManager` is unchanged outside the planner namespace migration; existing callers of the four orchestrator methods continue to work without modification.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): introduce LayoutReconciler with DesiredLayout for snapshot-driven restore paths
The previous commit split the planners into LayoutSolver and PendingLedger by temporality. With that boundary in place, the next consolidation step became viable: a single value type that captures "what the menu bar should look like", along with one composition point that decides "what is the next move toward that". Before the temporality split, this approach was blocked because `pendingReturnDestinations` and the`waitForRelaunch` sentinel could not fit into a single `DesiredLayout` without smuggling state through a side channel. After it, the pending-relocation ledger is already a separate concern, so `DesiredLayout` can stay focused on the layout question. This change adds that abstraction and migrates the two snapshot-driven paths that benefit from it.
Adds `Thaw/MenuBar/MenuBarItems/LayoutReconciler.swift` (193 lines). Defines three new value types: `DesiredLayout` (per-section ordered identifiers, pinning, `NewItemsPlacement` fallback) which is structurally the same whether built from a profile spec or from `savedSectionOrder`; `ObservedLayout` which packages the inputs the orchestrator already pre-computes (items, control items, section-by-windowID map, actively-shown tags); and `RestoreMove` which unifies the cross-section and within-section move shapes the underlying `LayoutSolver` planners emit. Defines `LayoutReconciler` as a static-only `enum` with two entry points: `nextRestoreMove(desired:observed:hasAlwaysHiddenSection:)` composes `planRebalanceMove` and `planWithinSectionReorder` with cross-section precedence, returning a single `RestoreMove` or nil; `unmanagedPlacementPlan(desired:unmanagedUIDs:currentUIDs:)` is a thin `DesiredLayout`-shaped wrapper around `planUnmanagedPlacement`. The reconciler owns no state and adds no new decision logic; it is purely a higher-level coordinator over the existing pure planners.
Migrates `restoreItemsToSavedSections` to use the reconciler. The previous structure ran `planRebalanceMove` for cross-section work and then `planWithinSectionReorder` as a separate fallback, each followed by its own destination-resolution switch and its own move-execution block. Both branches collapse into a single `LayoutReconciler.nextRestoreMove` call; the orchestrator then unpacks the returned `RestoreMove` into `(movingItem, destination, fromSection, toSection, updateSavedOrderSlot)` and runs one execute block. Destination resolution (anchor uid → live `MoveDestination`, with section-boundary fallback) is factored into a single inline `resolveDestination` helper. Net reduction: roughly 50 lines, and the function reads as one decision-then-execute flow instead of two parallel branches. Behavior is preserved including the cooldown gate, the 500 ms settle wait, the one-move-per-call contract, and the cross-section-only saved-order slot update.
Migrates `applyProfileLayout`'s unmanaged-items block to the same reconciler. Where it previously called `LayoutSolver.planUnmanagedPlacement` with four parameters built ad-hoc from `savedSectionOrder` and `newItemsPlacement`, it now constructs a `DesiredLayout` once and delegates to `LayoutReconciler.unmanagedPlacementPlan`. Profile-spec items and saved-restore items now share the same input shape; the duality that previously required two parameter lists is gone at the call site.
Adds `ThawTests/LayoutReconcilerTests.swift` with 7 characterization tests: cross-section precedence over within-section in `nextRestoreMove`; within-section fires after cross-section returns nil; nothing-to-reconcile returns nil; empty desired and observed return nil; `unmanagedPlacementPlan` favors saved positions; falls back to `.newItemDefault` for unseen items; uses `.newItemAnchored` when the `NewItemsPlacement` anchor is present in current. All 7 pass.
`relocateNewLeftmostItems` and `relocatePendingItems` are not migrated. The leftmost path has special-case logic (Thaw-icon recovery, non-hideable system-item recovery, newness reconciliation against `knownItemIdentifiers`) that does not fit the "make observed match desired" reconciler shape. The pending path lives in `PendingLedger` per the previous commit's temporality split, where it belongs. Keeping both as distinct orchestrators preserves the rule that each path's pathology should not smear across all triggers.
Pure refactor: no behavior change, no public API change. Full `ThawTests` suite goes from 626 to 634 (+8: the 7 new `LayoutReconcilerTests` plus a single incidental counter difference that surfaces under the reorganized assertion shape). The earlier planner-extraction, position-awareness, anchor-unification, position-preservation, and temporality-split tests all continue to pass identically.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): centralize destination resolution on LayoutReconciler
After the previous commit introduced `LayoutReconciler` as the composition layer over `LayoutSolver` planners, two near-duplicate code paths still resolved abstract `LCSPlannedDestination` values into concrete `MoveDestination` values: an inline closure inside `restoreItemsToSavedSections` and a 25-line switch inside the LCS phase 2 of `applyProfileLayout`. Two byte-identical section-boundary helpers (`sectionBoundaryDestination` instance method and `boundaryDestination` static mirror) also coexisted; the static mirror was only added so the inline closure could call it from a nonisolated context. Both duplications survived only because each refactor step introduced one helper and the prior step kept the other for backward compatibility. This change collapses them onto `LayoutReconciler` as the single bridge between LayoutSolver's abstract decisions and the move primitive's MenuBarItem-anchored inputs.
Adds `LayoutReconciler.resolveDestination(_:items:controlItems:fallbackSection:) -> MenuBarItemManager.MoveDestination`. Resolves an `LCSPlannedDestination` against a live items list: `.leftOfUID(anchorUID)` becomes `.leftOfItem(anchor)` when the anchor is found and movable, `.rightOfUID` becomes `.rightOfItem`, `.sectionBoundary(section)` resolves directly via `boundaryDestination`. Falls back to the section boundary for the supplied `fallbackSection` when the named anchor uid has disappeared mid-cycle.
Adds `LayoutReconciler.boundaryDestination(for:controlItems:) -> MenuBarItemManager.MoveDestination`. Canonical implementation of the "place at the section's control item" decision, identical in behavior to the two helpers it replaces.
Migrates the two consumers in `MenuBarItemManager.swift`. The inline `resolveDestination` closure inside `restoreItemsToSavedSections` is deleted; both `.crossSection` and `.withinSection` branches of `RestoreMove` now call `LayoutReconciler.resolveDestination(_:items:controlItems:fallbackSection:)` directly. The 25-line switch inside `applyProfileLayout`'s LCS phase 2 (lines 6189-6214 pre-change) collapses to one `LayoutReconciler.resolveDestination` call plus a one-line `fallbackSection` lookup.
Deletes the two duplicate boundary helpers from `MenuBarItemManager`: `sectionBoundaryDestination(for:controlItems:)` (private instance) and `boundaryDestination(for:controlItems:)` (`nonisolated private static`). Both had byte-identical bodies; only the second was reachable from planner-fallback paths, and only the first existed before this refactor began. Neither has a caller after the migration.
Adds 8 characterization tests in `ThawTests/LayoutReconcilerTests.swift`: four cases for `resolveDestination` (`.leftOfUID` with anchor present, `.rightOfUID` with anchor present, anchor-missing fallback, `.sectionBoundary` ignoring `fallbackSection`) and four cases for `boundaryDestination` (`.visible`, `.hidden`, `.alwaysHidden` with AH control present, `.alwaysHidden` without AH control falling back to hidden).
Pure refactor: no behavior change, no public API change. `MenuBarItemManager.swift` shrinks from 6,543 to 6,462 lines (net `-81`); `LayoutReconciler.swift` grows from 193 to 255 lines (net `+62`); the suite goes from 634 to 641 tests, all passing. Together the change is `-19` net lines of production code plus a stronger test net and one less concept (anchor resolution) duplicated across files.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): extract applyProfileLayout helpers and map layout coordination flags
Splits two pure blocks out of `applyProfileLayout` so the orchestrator reads as a sequence of named phases. The startup-settling wait loop becomes `waitForStartupSettlingToEnd()`; the three-pass unmanaged-item insertion (and its four local index helpers) becomes `LayoutReconciler.applyUnmanagedPlacementsToDesired`, a pure data transform sitting alongside the existing `unmanagedPlacementPlan`. Adds `// MARK:` headers at each execution-phase boundary inside `applyProfileLayout` (settling gate, persist and arm flags, expose sections, discover items, unmanaged placement, notch overflow, strategy choice, full-sort vs LCS execution, finalize) so the ~900-line orchestrator can be navigated without scrolling. The notch overflow, full-sort, and LCS sub-paths remain inline because each is tangled with async move primitives and live Window Server state; extracting them would mostly reshuffle 8-12 parameters rather than simplify.
Adds a `// MARK: - Layout coordination state` block above the in-flight flag cluster that maps each flag to one of three coordination concerns: cache-cycle gating (`isResettingLayout`, `isRestoringItemOrder`, `isApplyingProfileLayout`, `suppressNextNewLeftmostItemRelocation`), startup settling (`isInStartupSettling`, `settlingDeadline`, `settlingExpectedBundleIDs`, `settlingKind`), and active-profile re-sort (`activeProfileLayout`, `activeProfileItemIdentifiers`, `profileSortedItemIdentifiers`, `profileResortTask`). The flags are not collapsed into a single token because their semantics depend on AX-timing behavior the unit tests cannot exercise; a future consolidation step needs manual smoke testing on real hardware first.
Behavior preserved: full test suite green with no regressions.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): resolve nil-PID widgets via marker-window pairing on macOS 26
On macOS 26 some widgets (Little Snitch's agent observed in the wild) have their NSStatusItem hosted by Control Center at the AX layer and do not publish an `AXExtrasMenuBar` of their own. The spatial CG-to-AX resolver in `SourcePIDCache` cannot find a per-app extras child for them, `sourcePID` stays nil, and the namespace falls back to `com.apple.controlcenter`. The widget's identifier collapses to `com.apple.controlcenter:Item-0`, colliding with Apple's real Control Center items at `Item-19`/`Item-20`/`Item-21` and with any other widget hit by the same failure; the within-section reorder planner then sees the live widget as misplaced and moves it to an arbitrary spot in the visible bar.
Structurally, every NSStatusItem-style widget also publishes a SECOND CG window in the items-only list whose title is the widget's bundle identifier (verified empirically for `at.obdev.littlesnitch.agent`, `com.rogueamoeba.soundsource`, `com.wireguard.macos`, `org.eduvpn.app`, `com.lighting.huesync`, `pl.maketheweb.cleanshotx`, and others). This marker window has the same `(width, height)` as the on-screen icon, but its position is non-deterministic across launches and can even sit on a different display than the icon. Adds a post-resolution pass inside `SourcePIDCache.pidBody` (in the XPC, where `allWindows = WindowInfo.createMenuBarWindows(option: .itemsOnly)` already spans every display rather than the main app's per-call, per-display window list) that, for each still-unresolved on-screen icon whose own title is NOT bundle-ID-shaped (so genuine icons like `Item-0` pair, but two markers never pair with each other), looks for the unique marker window with matching size and synthesizes the `sourcePID` by either using the marker's CG-layer `owningApplication.processIdentifier` (preferring it when the resolved bundle is neither `com.apple.controlcenter` nor `com.stonerl.Thaw`) or by looking up `NSRunningApplication.runningApplications(withBundleIdentifier: marker.title).first`. Multi-match cases (two unresolved icons sharing a width with two markers) are skipped to prevent misattribution. Thaw's own control items (`Thaw.ControlItem.*`) and the Thaw self-registration window are excluded from the markers list, and the resolution path rejects any result that resolves back to Thaw, so Thaw's PID can never be attributed to a third-party widget. Resolved PIDs are stored directly into `state.pids` so the main app receives them through the normal XPC return path; the marker itself is left unresolved and is filtered out as off-screen elsewhere in the pipeline. Little Snitch's identifier becomes `at.obdev.littlesnitch.agent:Item-0` instead of `com.apple.controlcenter:Item-0`, and the spurious within-section reorder no longer fires.
Adds a diagnostic dump in `SourcePIDCache.pidBody` that fires when any window remains unresolved after the AX batch loop and the marker-pair pass. It logs each unresolved window's CG owner (`bundleIdentifier:pid=N`), `ownerName`, and the closest AX child frame across all apps with distance; dumps the AX children of every app that does publish an extras bar; and reports the presence or absence of probe bundles in `NSWorkspace.runningApplications`. This is what let us distinguish the actual macOS 26 failure mode (the widget's own app missing `AXExtrasMenuBar` while Control Center's extras bar contains a frame-matching child that the resolver's `isEnabled` filter rejects) from alternative theories such as the app not being enumerated, the spatial match being a near-miss, or the marker living on a different display than the icon. Adds `bundleIdentifier` and `localizedName` properties to `CachedApplication` to power the diagnostic. The diagnostic re-walks AX children so it only fires when there are unresolved windows; on a clean cycle it is silent.
Enables `DiagnosticLogger.shared.isEnabled = true` in `MenuBarItemService/main.swift` under `#if DEBUG`. The XPC service previously never enabled file logging, so `[SourcePIDCache]` debug lines flowed only to OSLog and never landed in `~/Library/Logs/Thaw/thaw_*.log`. This had hidden the diagnostic from earlier capture attempts and made the failure mode harder to pin down than it should have been.
Fixes a latent race in `DiagnosticLogger.openLogFile` that the XPC change above uncovered. The previous implementation called `FileManager.default.createFile(atPath:, contents: nil)` followed by `FileHandle(forWritingTo:)` and `seekToEndOfFile()`, but when the main app and the XPC service both enable logging during startup they land on the same `thaw_<HH-mm-ss>.log` filename, and `FileManager.createFile` truncates an existing file at the path. The XPC's call wiped the main app's header and subsequent writes from each process's independent `FileHandle` offset overwrote each other. The visible symptom on every recent debug-build launch was a file containing only the XPC's entries with the main app's `[MenuBarItemManager]`, `[AppState]`, and `[MenuBarItem]` lines silently destroyed. Switches the open path to POSIX `open(fileURL.path, O_WRONLY | O_APPEND | O_CREAT, 0o644)` wrapped in a `FileHandle(fileDescriptor:, closeOnDealloc: true)` so the kernel atomically positions each write at end-of-file (safe between processes on local filesystems for writes smaller than `PIPE_BUF`, well above any single log line) and so an existing file is opened rather than truncated. The header gains a `Process: <name>` line to distinguish the two streams; per-line timestamps preserve chronological order. Removes the now-redundant `seekToEndOfFile()` call, which would have been actively unsafe with multiple writers.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): converge layout restore loop and protect saved order during profile apply
Three coordinated fixes to the cache-cycle / restore / profile-apply state machine, observed via two logs: an ordinary relaunch where the saved layout failed to fully restore, and an auto-profile-switch where the saved order got clobbered mid-apply.
First fix: thread `skipRecentMoveCheck` through to `restoreItemsToSavedSections` and gate the 5s cooldown on it. `restoreItemsToSavedSections` is a one-move-per-call function; after each move the orchestrator schedules a recache via `Task { ... cacheItemsRegardless(skipRecentMoveCheck: true) }` (line 1954), which re-enters and lets the planner emit the next move until `LayoutReconciler.nextRestoreMove` returns nil. The
5-second cooldown at the top of the function (originally there to suppress cascading restores when app relaunch waves trigger rapid cache cycles) was blocking that follow-up: a move had just completed ~320ms earlier, so the cooldown rejected the loop's own next step and `didRestoreSections` came back false. The orchestrator then cleared `isRestoringItemOrder`, fell through to `uncheckedCacheItems`, and
`saveSectionOrder` persisted the partial state as the new target. From there the bad state was sticky because the saved order matched what was on screen, leaving nothing for the next launch to fix. The flag already meant "this is a follow-up cycle, bypass the recent-move guards" for the save-side check at line 1696; this commit extends that contract to cover the restore-side cooldown too. External triggers still call
with `skipRecentMoveCheck: false` so the cascade protection is unchanged; only the loop's own recache bypasses it. Effect: after the first cross-section move the loop re-enters, the reconciler emits the next misplaced item, and so on until convergence. Observed in the wild on a user with a 15-entry saved layout: previously only `notion.id` got restored from visible to hidden and `saveSectionOrder` wrote a `["visible":
11, "hidden": 4]` snapshot, dropping the original `["visible": 8, "hidden": 7]` intent.
Second fix: add `!isApplyingProfileLayout` to the `saveSectionOrder` guard at line 1655. `applyProfileLayout` sets both `isApplyingProfileLayout = true` and `isRestoringItemOrder = true` and relies on the latter to suppress saves of in-flight intermediate state. But when a nested `cacheItemsRegardless` runs during the profile apply (e.g., the recache scheduled after a successful applyProfileLayout move), it calls
`restoreItemsToSavedSections`; if that nested restore returns false for any reason (a failed move, the planner returning nil), the cache-cycle path at line 1966 unconditionally resets `isRestoringItemOrder = false`, clobbering the outer profile apply's flag. The next line falls through to `uncheckedCacheItems`, whose save guard now passes, and the partial profile-apply state gets persisted as the new saved order.
Observed in the Alienware-profile auto-switch log: a failed displaylink restore inside a recache at 14:19:34.987 triggered a `["visible": 20, "hidden": 5, "alwaysHidden": 2]` snapshot at 14:19:34.991, overwriting the user's `["visible": 6, "hidden": 19, "alwaysHidden": 2]` intent before `applyProfileLayout` had even finished its 6 moves. Adding the profile-apply check to the save guard makes the save-side gating immune
to the nested clobber; the partial state stays in memory only and the post-profile-apply save reflects the profile's intended layout.
Third fix: add `guard !isApplyingProfileLayout else { return false }` at the top of `restoreItemsToSavedSections`. Without this, a cache cycle that fires during the profile apply window can plan restore moves that directly undo what `applyProfileLayout` is doing. The Alienware log shows the back-and-forth thrash: displaylink moved to hidden by restore at 14:19:34.351, then back to visible at 14:19:37.285; beyondtrust, knollsoft.Hookshot, and cisco.secureclient all see follow-up within-section reorders after the profile apply completed because the live state was modified twice. The save guard above prevents the corruption; this guard prevents the wasted moves and visible flicker. With both gates in place, `applyProfileLayout` has an exclusive window from `isApplyingProfileLayout = true` (line 5361) until its `defer` clears it at exit: no restore moves planned, no saved-order writes; the cache cycle resumes its normal duties only after the profile apply completes.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): preserve Layout Bar drags by narrowing the restore-cooldown bypass
The previous commit threaded `skipRecentMoveCheck` through to `restoreItemsToSavedSections` so the restore loop's own follow-up recache could bypass the 5-second cooldown and converge to the saved layout. That bypass was too broad: the Layout Bar drag-and-drop handler also calls `cacheItemsRegardless(skipRecentMoveCheck: true)` after a manual drag (for the save-side cooldown at line 1696, so the user's drag is persisted promptly), and the same flag was making the post-drag recache fall through into `restoreItemsToSavedSections` with the restore cooldown bypassed. The restore then read the current saved order, saw the just-dragged item in a different section, and emitted a cross-section move that undid the drag before `saveSectionOrder` could capture the new state. The visible symptom was that dragging an item between sections in Settings reverted within a few hundred milliseconds.
Captures `isRestoringItemOrder` at the entry of `cacheItemsRegardless` before the line that sets it to `true`, into a local `isRestoreLoopFollowUp`, and threads that value through to `restoreItemsToSavedSections` in place of `skipRecentMoveCheck`. The restore loop schedules its follow-up `Task { ... cacheItemsRegardless(skipRecentMoveCheck: true) }` while the outer call's `isRestoringItemOrder` is still `true` (the Task clears it only after its recache returns), so the loop's follow-up enters with the flag already set and `isRestoreLoopFollowUp` resolves to `true`; the cooldown bypass fires and the loop converges as before. Every other caller (Layout Bar drag, ordinary cache cycle, settling-driven cycle) enters with the flag `false`, `isRestoreLoopFollowUp` resolves to `false`, and the cooldown applies normally so the recent manual move stays put and `saveSectionOrder` persists the new state.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): preserve original section on save when a temporarily-shown item's app quits
`temporarilyShow` records `pendingRelocations[tagIdentifier] = originalSection` and `pendingReturnDestinations[tagIdentifier]` before moving an item from hidden / always-hidden into visible, then installs a `TemporarilyShownItemContext`. While the context exists, two protections work together: `uncheckedCacheItems` caches the item at its return destination rather than its live visible position, and `saveSectionOrder` is gated entirely by `temporarilyShownItemContexts.isEmpty`. The rehide path drops the in-memory context once the rehide succeeds; it also drops the context when the rehide gives up (the cap-reached branch overwrites `pendingRelocations` with a `waitForRelaunch:` sentinel) or when the user explicitly abandons the return via `removeTemporarilyShownItemFromCache`. If the app quits while the item is still physically in visible and the rehide gives up before the window disappears, the protective context is gone but the item's live cache position is still visible, the `temporarilyShownItemContexts.isEmpty` gate now passes, and `saveSectionOrder` persists the item in visible — overwriting its real saved slot in the original section. On the next launch the saved order says visible, so the restore complies, and the original placement is lost.
`pendingReturnDestinations` and the `waitForRelaunch:` sentinel in `pendingRelocations` outlive the in-memory context and are the canonical "this item's true home is elsewhere" signal that survives the rehide-gives-up window. `saveSectionOrder` now consults both: the union of `pendingReturnDestinations` keys and `pendingRelocations` entries whose value starts with `waitForRelaunchPrefix` forms `pendingRehideTagIDs`, and items whose `tag.tagIdentifier` is in that set are excluded from both the per-section `currentInSection` list and the top-level `allCurrentIdentifiers` / `allCurrentBaseIdentifiers` sets. `LayoutSolver.planSectionOrder` then sees them as closed apps and applies its closed-app preservation logic, splicing them back into their old saved-section slot rather than the live visible one. When the app relaunches, `relocatePendingItems` moves the item back to its original section and clears both `pendingReturnDestinations` and `pendingRelocations`; the item drops out of `pendingRehideTagIDs` on the next save and participates normally. The existing `temporarilyShownItemContexts.isEmpty` gate at line 1657 is left in place — it remains the primary protection during the in-flight window when no save should fire at all — and the new exclusion only takes effect during the recovery window between rehide-give-up and app-relaunch.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): drive XPC PID scan via an unresolved window so mid-session arrivals resolve
`SourcePIDCache.pidsBody` is the batch entry point that fronts the AX traversal: it checks whether any window in the batch is unresolved and, if so, invokes `pidBody` once to populate `state.pids` for every resolvable window in a single scan. The invocation used `windows.first`, but `pidBody` has a cache-hit early return at the top (line 292) that exits before reaching the scan body. Whenever the first window in the batch was already cached (i.e. any time after the session's first cycle resolved the leftmost item), `pidBody(windows.first)` returned the cached PID immediately and the scan never ran, even though `needsScan` correctly identified that other windows in the batch still needed resolution.
The bug was latent for two days after the batching commit landed (`0813e6e1`, May 13). Pre-macOS-26 the AX spatial pass resolved virtually every widget in the very first scan at session start, after which the cache was populated for the rest of the session and no further scans were needed; the broken re-trigger path was invisible because nothing ever needed it. On macOS 26 some widgets (anything whose owning app does not publish `AXExtrasMenuBar`) depend on the marker-pair fallback inside `pidBody`'s scan body, and that fallback can only catch a widget when its windowID is present in `allWindows` during the scan. New apps launching mid-session (LittleSnitch, WireGuard, Strongbox observed in the wild) introduce a fresh nil-PID windowID into the batch, but the first window of the batch is now always some old cached entry, so the scan stays dormant and the new widget's identifier collapses to `com.apple.controlcenter:Item-0:N` for the rest of the session. `relocateNewLeftmostItems` then short-circuits on `.unresolvedSourcePID` and `restoreItemsToSavedSections` cannot match the unstable identifier against the saved order, so the icon stays at macOS's default leftmost placement.
Replaces `windows.first` with `windows.first(where: { state.pids[$0.windowID] == nil })`. The new selection always passes an actually-unresolved window to `pidBody`, which drops past the cache-hit return into the scan body where both the AX spatial pass and the marker-pair fallback can run. The `needsScan` boolean is folded into the `first(where:)` lookup itself: when every window in the batch is cached the closure finds nothing and `pidBody` is not invoked, matching the original early-exit behavior. Session-start behavior is unchanged (cache empty, first window also the first unresolved). Mid-session new-app arrivals now get the scan they always needed, so their sourcePID resolves on the same cycle their windowID appears and the existing relocate / restore paths handle the placement.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* test(menubar): characterize recent state-machine fixes with extracted pure helpers
Adds 36 regression tests across five files for fixes landed in the last few commits, extracting three pure helpers in `LayoutSolver` and one in a new `Shared/Utilities/MarkerPairResolver` so the algorithms can be characterized at the spec level without the surrounding orchestrator state.
Extracts `LayoutSolver.shouldPersistSavedOrder(...)`, `LayoutSolver.pendingRehideTagIdentifiers(...)`, and `LayoutSolver.selectWindowForBatchScan(...)` as pure `nonisolated static` helpers consumed by `MenuBarItemManager.swift`. The save guard at the `uncheckedCacheItems` save path now reads as one named call instead of an inline five-clause boolean; the pending-rehide identifier computation reads as one call instead of an inline set-union; and the cache-cycle batch-scan window selection routes through a generic helper that pidsBody's algorithm matches. Each helper is pure over its inputs so the surrounding live state is irrelevant to the characterization.
Creates `Shared/Utilities/MarkerPairResolver.swift` and migrates the inline marker-pair pass from `SourcePIDCache.pidBody` onto it. The resolver takes injected `pidToBundleID` and `bundleIDToPID` closures so the XPC service wires them to `NSRunningApplication` while tests supply deterministic lookups; `extractMarkers` is a separate pure helper that handles the Thaw-self and `Thaw.ControlItem.` exclusions. The XPC call site is now a single `MarkerPairResolver.resolve(...)` invocation against two pre-built input arrays. The file is placed in `Shared/` so both the Thaw target and the MenuBarItemService XPC target compile it; ThawTests reaches it via `@testable import Thaw`.
New tests:
- `PlanLeftmostMoveTests.testNewWindowIDWithUnresolvedSourcePIDStillShortCircuits` pins the current `.unresolvedSourcePID` behavior for the new-windowID-with-nil-sourcePID scenario observed in the wild so any future loosening of the leftmost-relocator short-circuit becomes a deliberate test change.
- `ShouldPersistSavedOrderTests` (7 cases) covers the in-flight-flag truth table including the `isApplyingProfileLayout` gate that blocks saves while a profile apply is in flight.
- `PendingRehideTagIdentifiersTests` (7 cases) covers the post-rehide-give-up filter including `pendingReturnDestinations`, the `waitForRelaunchPrefix` sentinel, anchored prefix matching, set union of both sources, exclusion of non-sentinel `pendingRelocations` values, and deduplication across both sources.
- `SelectWindowForBatchScanTests` (7 cases) covers the first-unresolved selection that drives the XPC batch scan, including the realistic mid-session shape (first window cached, later windows unresolved) which used to skip the scan entirely.
- `MarkerPairResolverTests` (14 cases) covers the canonical observed scenario, the owning-PID-preferred path, Control Center and Thaw exclusions on both resolution paths, multi-match rejection, self-pair rejection, size-mismatch rejection, generic-titled icon filtering so two markers cannot pair with each other, plus `extractMarkers` exclusions for Thaw control items and the Thaw self-registration window.
Total suite: 677 pass, 0 fail (up from 641). No behavior change; the refactored call sites compile to the same logic as before.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): consolidate saved-order restore into the bulk profile-apply path
The non-profile restore previously drove its own one-move-per-cache-cycle loop through restoreItemsToSavedSections, leaning on an incremental planner pair (planRebalanceMove for cross-section moves and planWithinSectionReorder for intra-section drift) composed by LayoutReconciler.nextRestoreMove. The profile path already did a single bulk reconciliation through applyProfileLayout, including LCS-based intra-section moves and notch-overflow planning, so the two routes maintained near-duplicate logic for the same goal: bring the live layout in line with savedSectionOrder.
The new applySavedLayout entry point on MenuBarItemManager builds an itemSectionMap from savedSectionOrder and dispatches into applyProfileLayout with source: .savedOrder. applyProfileLayout now branches on an ApplySource enum so the profile-only state arming (pinning rewrites, activeProfileLayout / activeProfileItemIdentifiers, isApplyingProfileLayout, profileResortTask) and the matching teardown (updateProfileSortedSnapshot, flag clear) only run for actual profile activations; the saved-order path skips them entirely and instead reuses the profile pipeline's classification, LCS, full-sort, and notch-overflow stages. Entry guards for the saved-order path live on applySavedLayout itself: empty savedSectionOrder, suppressNextNewLeftmostItemRelocation, isApplyingProfileLayout, the 5s move cooldown, an actual window-ID change versus the previous cache cycle, and at least one saved tag present in the current snapshot.
With both routes converged, the incremental planners and their composer are gone. LayoutSolver no longer carries planRebalanceMove, planWithinSectionReorder, or the RebalanceMove / WithinSectionMove move types. LayoutReconciler no longer exposes RestoreMove or nextRestoreMove; the cache cycle calls applySavedLayout directly and the isRestoreLoopFollowUp capture-at-entry plumbing in cacheItemsRegardless is removed. anchorDestination remains in use by the profile-route unmanaged-placement path and by the reconciler when resolving saved positions into concrete destinations, so its doc comment is updated to reflect the smaller callsite set. The associated planner tests (PlanRebalanceMoveTests, PlanWithinSectionReorderTests) and the nextRestoreMove cases in LayoutReconcilerTests are deleted; the remaining 657 ThawTests still pass with zero failures.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): persist profile state only after the bulk apply succeeds
`applyProfileLayout`'s profile-only state arming and clearing now live in three private helpers (`armProfileState`, `updateProfileSortedSnapshot`, `clearProfileState`) that each internal-guard on `case .profile = source`. The four scattered `if case .profile = source` branches at the entry, the two no-op early-returns, and the exit point are gone; callsites invoke the helpers unconditionally and the helpers no-op for `.savedOrder`. `cacheItemsRegardless` no longer arms `isRestoringItemOrder` around its `applySavedLayout` call because `applyProfileLayout`'s body already owns that flag's lifetime through its `defer` block; the previous double-arming on the success path and arm-then-clear on the rejection path are removed.
Task cancellation now propagates through the gaps the earlier pass missed. Phase 0 gains an explicit `Task.isCancelled` return after `waitForStartupSettlingToEnd` so a settling-wait that races a `layoutTask.cancel()` from a display-reconfiguration or Focus-filter switch bails before any state arms. Phase 7b sub-phase 1's AH-control placement and both per-item cross-section fallback loops gain matching guards, matching the existing pattern in Phase 7a's full-sort and Phase 7b's main LCS loop.
Disk persistence of `pinnedHiddenBundleIDs`, `pinnedAlwaysHiddenBundleIDs`, and `savedSectionOrder` is no longer tied to the entry-time state arm. `armProfileState` only mutates in-memory state; a new `persistProfileStateOnSuccess(source:)` helper writes `UserDefaults` and is called at each success exit (Phase 7a early-return, Phase 7b early-return, and Phase 8 normal exit gated on `Task.isCancelled`). A crash, SIGKILL, or mid-apply cancellation between the arm and the Phase 7 commit now leaves `UserDefaults` reflecting the previous profile rather than an unexecuted intent. Profile files on disk are unaffected; only Thaw's `UserDefaults`-backed live-state writes were ordering-reliant.
`applySavedLayout`'s six entry guards each log a distinct `.debug` line per rejection reason (`savedSectionOrder` empty, `suppressNextNewLeftmostItemRelocation` armed, profile apply in flight, within the 5 s move cooldown, no windowID change since the previous cycle, no saved items currently present) so a future "Thaw stopped restoring my layout" report is diagnosable from a single log. A comment block above the `windowIDsChanged` gate enumerates the four windowID-set transitions (pure-add, app-quit, app-relaunch, WID-recycling) and which subsystem owns each.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* feat(menubar): trigger savedOrder restore on layout drift, not just windowID changes
`applySavedLayout` now advances past the trigger gate on either of two independent signals. The existing `windowIDsChanged` check still fires on app-quit and app-relaunch (a previous windowID missing from the current set). A new `layoutDiverged` check fires when at least one movable, hideable item whose baseID appears in `savedSectionOrder` is currently in a different section than where it was saved. Divergence is computed lazily, only when the windowID signal didn't already advance the gate, so the app-quit happy path is unchanged in cost.
The new signal closes three gaps the prior windowID-only trigger left open. Cold boot for users without an auto-applied profile previously left the bar in macOS-default-spawn order until the first app quit during the session, because the first cache cycle has `previousWindowIDs` empty and subsequent cycles only see additions; the layout-divergence check now reasserts the saved order on the first cycle where bounds disagree with `savedSectionOrder`. Ambient drift from third-party menu bar utilities, Stage Manager toggles, and screen lock/unlock cycles previously went uncorrected because windowIDs stay stable while sections shift; the divergence trigger catches these as well. WindowID recycling (same WID, different item) is still uncovered by either signal, but in practice this would also surface as a section mismatch and so is now indirectly covered.
The divergence check reads item bounds directly from the items array the caller already populated via `getMenuBarItems`; no per-item AX round-trip through `CacheContext`. Items that straddle a control-item boundary are deliberately not classified, to avoid false positives during transient section show/hide animations. Multi-instance baseIDs use last-write-wins in the expected-section map, which can false-positive when one app has instances split across sections in the saved order, but the bulk apply early-returns when no moves are required so the cost is one extra "current order matches desired" log per affected cycle. `applySavedLayout`'s dispatch log line now names which trigger fired (`windowID change` or `layout divergence`) so log forensics can tell the two cases apart. `applySavedLayout` gains a `controlItems` parameter so the divergence classifier can compare item bounds against control-item X positions; the single caller in `cacheItemsRegardless` already had the value on hand.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): exclude the visible control item from unmanaged placement
The `.savedOrder` path's Phase 4 filter excluded `hiddenCtrlUID` and `ahCtrlUID` but not `visibleCtrlUID`. Combined with `saveSectionOrder`'s deliberate omission of control items from `savedSectionOrder` (line 471, `where !item.isControlItem`), this meant `Thaw.ControlItem.Visible` was never in `desiredSet` on entry to Phase 4, never excluded by the filter, and therefore deemed an unmanaged item every cache cycle. `planUnmanagedPlacement` routed it through the user's `NewItemsPlacement` rule (in one field reproduction, `leftOfAnchor=com.anthropic.claudefordesktop:Item-0`), and the LCS planner emitted a one-move sequence on every windowID change to drag the Thaw icon to the configured anchor. The user's manual fix never stuck because `saveSectionOrder` doesn't track control items, so subsequent cycles re-ran the same placement and dragged the icon back.
The fix extracts the inline filter into a new pure helper `LayoutSolver.partitionUnmanagedUIDs(currentFlat:desiredUIDs:hiddenCtrlUID:ahCtrlUID:visibleCtrlUID:)` that uniformly excludes all three Thaw control items and preserves input order (the downstream LCS planner relies on iteration order for placement application). The Phase 4 call site in `applyProfileLayout` is now a single delegating invocation; the cross-file invariant between `saveSectionOrder` and the unmanaged filter is documented in the helper's docstring so future changes that re-introduce the same omission read against an obvious contract.
`ThawTests/PartitionUnmanagedUIDsTests.swift` adds five characterisation cases: all three control items excluded (the case the original filter missed), `nil` control UIDs tolerated for alwaysHidden-disabled configurations, items in `desiredUIDs` excluded, input order preserved, and an empty `currentFlat` returning an empty result. Suite count goes from 657 to 661 passing.
A per-uid diagnostic remains inside the same Phase 4 block, logging which item the planner deemed unmanaged and which placement strategy (`.saved`, `.newItemAnchored`, or `.newItemDefault`) fired with its parameters. It only emits when there are unmanaged items to place, so steady-state cycles incur no log cost. This is the line that caught the bug in field logs and stays in as the planned input format for the future log-replay harness.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): defer the post-apply cache refresh so itemCache stays fresh
Every `applyProfileLayout` exit point ended with an inline `await cacheItemsRegardless(skipRecentMoveCheck: true)` (Phase 8 plus three Phase 7 early-returns). Because `applyProfileLayout` is itself awaited by `cacheItemsRegardless` via `applySavedLayout`, the outer call still holds its serial `cacheGate` at the point of the inline recursive recache, so the call was rejected with "serial cache operation already in progress, skipping" and `itemCache` stayed stale. The user-visible symptom: apps that had just quit continued to appear in the Settings Layout pane, `ThawBar`, and Search until something else triggered a non-`applySavedLayout` cache cycle.
The fix introduces a private `scheduleDeferredCacheRefresh()` helper that spawns a detached `Task`. The Task sleeps `uiSettleDelay` (300 ms), then calls `cacheItemsRegardless` followed by an `imageCache.performCacheCleanup()` + `imageCache.updateCacheWithoutChecks(...)` and an `objectWillChange.send()` notification. The body runs after the outer `cacheItemsRegardless` releases its gate, so the recache proceeds normally and `itemCache` gets refreshed against the post-apply WindowServer state. The pattern mirrors the existing relocate-path recache schedule. All four `applyProfileLayout` exit points (Phase 7a "current order matches desired", Phase 7b "lost control items after phase 1", Phase 7b "all items already in correct positions" / "completed with N control item moves", and Phase 8 normal exit) now route through this single helper instead of inline-awaiting `cacheItemsRegardless`.
A new `skipSavedLayoutApply` parameter on `cacheItemsRegardless` gates the `applySavedLayout` block. `scheduleDeferredCacheRefresh` passes `skipSavedLayoutApply: true` so the post-apply refresh never re-enters `applySavedLayout`. Without this gate the deferred refresh's `cacheItemsRegardless` re-dispatches `applySavedLayout`, schedules another deferred refresh, and the cycle never terminates because consecutive `getMenuBarItems` calls can return slightly different windowID sets (transient Apple Control Center widgets like `Clock` and `BentoBox`, plus Phase 2's section-expand/collapse cycle, churn windowIDs even when the visible item count is stable). The observed regression with no gate was the Thaw chevron flickering at ~500 ms cadence indefinitely. With the gate, the deferred refresh proceeds straight to `uncheckedCacheItems`, which updates `itemCache` and runs `saveSectionOrder`; the next user-triggered cycle still goes through `applySavedLayout` normally on a real windowID change.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(menubar): stop notched full-sort from re-sorting items already in order
`currentFlat` (the orchestrator's flat sequence of current items used by the planner) was being built in two passes: a section-iteration pass that filtered items via `isProfileItem`, followed by an explicit append of `hiddenCtrlUID` after the visible section and `ahCtrlUID` after the hidden section. The bug: `findSection` classifies `Thaw.ControlItem.Hidden` into `.visible` (the divider's bounds straddle its own boundary, so the midpoint fallback in `findSection` lands it in `.visible`) and `Thaw.ControlItem.AlwaysHidden` into `.hidden`. Both pass `isProfileItem` because they have `canBeHidden=true` and `isMovable=true`. The first pass therefore already appended each divider via its section, and the second pass appended it again. `currentFlat` ended up with each divider twice while `desiredFlat` (built from `itemOrder`, which excludes control items, plus a single explicit append per divider) had each divider exactly once.
The mismatch broke `planFullSortSequence`'s early-return: `currentFlat.filter { desiredSet.contains($0) } == desiredFiltered` was always false because the LHS had duplicate divider entries the RHS lacked, so the planner returned the full sequence even when the live bar already matched the saved order. The user-visible symptom on notched displays with `useLCSOnNotched=false` was the full-sort path moving all 26 menu bar items on every `applySavedLayout` dispatch, taking ~13 seconds per apply and animating every time the cache cycle triggered (which after the deferred-refresh fix is roughly every windowID change). The Phase 7b LCS path tolerated the duplicate because it explicitly strips the divider UIDs from `currentFlat` and `desiredFiltered` before LCS planning.
The fix filters `hiddenCtrlUID` and `ahCtrlUID` out of `sectionItems` during the first construction pass, so each divider lands in `currentFlat` exactly once via the explicit append after its section. With the duplicates gone, `planFullSortSequence`'s early-return fires correctly when the bar matches saved order, the Phase 7a path takes the "current order matches desired, skipping" branch, and `scheduleDeferredCacheRefresh` runs without any item moves.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* refactor(menubar): remove applyProfileLayout Phase 2 and renumber subsequent phases
`applyProfileLayout` previously opened with a Phase 2 block that called `section.show()` on every non-visible section to expose items for moves, paired with a `defer` block that restored every section to `.hideSection` and closed `iceBarPanel` on exit. The block was added during profile-feature work but `move()` and the relocate paths (`relocateNewLeftmostItems`, `relocatePendingItems`) move items without needing the destination section to be pre-exposed, so the only user-visible effect of Phase 2 was a chevron expand/collapse animation on every apply, including the now-common case where the planner determines no moves are needed and exits via an early-return. With Phase 2 removed, apply dispatches that find the bar already in correct order are invisible.
The remaining outer-pipeline phases are renumbered to close the gap: the old Phase 3 (discover items) becomes Phase 2; Phase 4 (unmanaged placement) becomes Phase 3; Phase 5 (notch overflow) becomes Phase 4; Phase 6 (execution-strategy choice) becomes Phase 5; Phase 7a / 7b (full-sort and LCS execution) become Phase 6a / 6b; Phase 8 (finalize) becomes Phase 7. Cross-references in `scheduleDeferredCacheRefresh` and `persistProfileStateOnSuccess` docstrings, the mid-Phase-6 cancellation comment in the finalize block, and the `skipSavedLayoutApply` rationale comment on `cacheItemsRegardless` are updated to match. The inner "Phase 1" / "Phase 2" sub-phase references inside the LCS execution block describe a separate naming convention (LCS cross-section vs within-section ordering) and are left alone.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
* fix(profile): persist useOptionClick, useLCSSorting, and enableOverflow in AdvancedSettingsSnapshot
AdvancedSettingsSnapshot was missing three properties that exist on AdvancedSettings: useOptionClickToShowAlwaysHiddenSection, useLCSSortingOnNotchedDisplays, and enableMenuBarItemOverflow. Profile capture wrote them to JSON as absent and profile apply silently left the live UserDefaults values in place, so toggling any of these three from the Advanced settings pane would persist across restarts even when the user expected the profile to be the source of truth on the next load.
The snapshot now captures all 15 published properties on AdvancedSettings; capture(from:) reads the three new fields off AdvancedSettings and apply(to:) writes them back. The CodingKeys enum lists the new keys and init(from decoder:) uses decodeIfPresent with the corresponding Defaults.DefaultValue fallback so existing profile JSONs without the new keys decode cleanly to their default values. Profile.init(from decoder:)'s fallback AdvancedSettingsSnapshot constructor (used when the entire advancedSettings block is missing from the JSON) is updated to pass the three new defaults.
* refactor: replace emdashes with semicolons in comments and log strings
Project convention forbids emdashes and endashes anywhere in source. LayoutSolver had three comment occurrences (one in planLeftmostMove's transient Control Center exclusion block, one in the planLCSMoveSequence anchor-stability docstring, one in planSectionOrder's saved-entry preservation comment) and ProfileManager had two (one Focus Filter fallback comment, one Focus Filter deactivation diag.info string) which used emdashes for sentence-joining. All five occurrences are replaced with semicolons, which preserve the same intent without violating the punctuation rule.
* fix(menubar): apply profile on launch, count chevron in budget, gate re-sort on section drift, persist chevron position
Profile load on Thaw startup previously only restored the menu-bar layout via the cache cycle's applySavedLayout path, which reads savedSectionOrder from UserDefaults and never touches general/advanced/hotkey/appearance/display settings. The settling-end hook in startSettlingPeriod now calls reapplyActiveProfile when an active profile is set and awaits the resulting layoutTask, so the full applyProfile chain (applySnapshot followed by applyProfileLayout) runs to completion before the cache cycle's restore fires. The profile is the source of truth on launch when one is bound to the display, instead of the live savedSectionOrder which can diverge from the profile spec across restarts (manual drags, unmanaged items inserted by NewItemsPlacement, etc.).
Notch-overflow budget under-counted profileBaseline by the chevron's bounds.width (about 12pt). isProfileItem returns true for the visibleControlItem so the nonProfileFootprint loop skipped it, and the chevron is absent from desiredFiltered (the planner does not constrain its position) so its width was never in uidWidths either. On notched layouts where the actual sum sits within ~12pt of the budget, this error tipped the math the wrong way and overflow silently skipped. The orchestrator now computes a chevronFootprint and subtracts it from availableWidth directly when the chevron sits in the visible region; desiredFlat stays untouched so the LCS continues to leave the chevron at whatever spatial position the user has chosen. The budget diag log gains a chevronFootprint=N field so future overflow misses can be triaged from the log.
The re-sort trigger at the start of cacheItemsRegardless previously dropped every profile-tracked item with a fresh windowID from profileSortedItemIdentifiers, scheduling a full re-sort even when the item's spatial section already matched the profile spec. Idle wake, AX rebinding, and other NSStatusItem lifecycle events that recreate the underlying window without changing position fired this path on each cache cycle, triggering a 3-or-more-move re-apply for items that were exactly where they should be. The drop is now gated on a section mismatch: when the item's current section matches the profile's expected section for that uniqueIdentifier, the item stays in the sorted snapshot and no re-sort is scheduled. Items whose current section cannot be determined (transient bounds during in-flight moves) fall through to the drop path, preserving the original conservative behaviour for ambiguous cases. The diag log changes from "with fresh windowID" to "at wrong section" so the new gate's intent is clear.
saveSectionOrder now persists the visibleControlItem (Thaw chevron) alongside profile-tracked app items via a new isPersistable predicate that admits item.tag == .visibleControlItem in addition to the existing "non-control with resolved sourcePID" path. The hidden and alwaysHidden control items remain excluded because their position is implicit (always at the section boundary) and they get inserted into desiredFlat at the section boundary regardless of saved order. The chevron persists at its current spatial index in the visible section so its user-chosen position survives Thaw restarts, instead of macOS's NSStatusItem autosave…1 parent 2a8301c commit e5d3556
32 files changed
Lines changed: 6159 additions & 970 deletions
File tree
- MenuBarItemService
- Thaw.xcodeproj
- ThawTests
- Thaw
- MenuBar/MenuBarItems
- Permissions
- Settings/Models
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
47 | 59 | | |
48 | 60 | | |
49 | 61 | | |
| |||
277 | 289 | | |
278 | 290 | | |
279 | 291 | | |
280 | | - | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
281 | 302 | | |
282 | | - | |
283 | | - | |
284 | | - | |
| 303 | + | |
| 304 | + | |
285 | 305 | | |
286 | 306 | | |
287 | 307 | | |
| |||
367 | 387 | | |
368 | 388 | | |
369 | 389 | | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
370 | 468 | | |
371 | 469 | | |
372 | 470 | | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
373 | 548 | | |
374 | 549 | | |
375 | 550 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
11 | 15 | | |
12 | 16 | | |
13 | 17 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
128 | 128 | | |
129 | 129 | | |
130 | 130 | | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
150 | | - | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
157 | 148 | | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
158 | 180 | | |
159 | 181 | | |
160 | 182 | | |
| |||
0 commit comments