fix(menubar): address review feedback for #889 - #892
Merged
diazdesandi merged 1 commit intoAug 5, 2026
Merged
Conversation
Signed-off-by: VailElla <63608800+VailElla@users.noreply.github.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
menubar
Contributor
|
Labels: All convention checks passed. ✅ cc @VailElla |
diazdesandi
merged commit Aug 5, 2026
fa965b6
into
thaw-app:fix/menubar-reliability
10 of 13 checks passed
diazdesandi
pushed a commit
that referenced
this pull request
Aug 14, 2026
Signed-off-by: VailElla <63608800+VailElla@users.noreply.github.com>
diazdesandi
added a commit
that referenced
this pull request
Aug 15, 2026
* fix(menubar): delete the full-sort path that rearranges the whole bar
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* refactor(image): centralize alpha channel access for transparency scans
Cherry-pick of a1bb8467 from feat/macos-27-experimental, so the 26/27
shared codebase does not diverge on image handling.
Introduce `AlphaChannelView` to encapsulate alpha channel access, buffer
validation, and pixel/row transparency checks. This centralizes bounds
validation, preventing potential out-of-bounds reads during image
processing, and simplifies the `TransparencyContext` and
`isTransparent(fastPath:)` implementations.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): stop failed move attempts from starving the timeout budget
Folded from 2 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(menubar): stop failed move attempts from starving the timeout budget
Signed-off-by: René Jiménez <diazdesandi@proton.me>
test(menubar): replay layout storm against the surviving planner
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(menubar): add experimental gesture geometry for item moves (default off)
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(diagnostics): log a section-order digest
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): keep LyricsX lyrics out of the new-items section
LyricsX titles its menu bar item with the lyric line currently on screen, and identity is namespace:title. Every song change — every line — minted a fresh uniqueIdentifier, so the item read as a brand-new arrival and was filed under the user's new-items section. Dragging the lyrics into visible could not stick, because the item that was moved stopped existing at the next line.
The volatile-title machinery already existed for iStat Menus, which has the same problem in a different shape, but it did not cover this. Two reasons:
1. The owner allowlist held exactly one bundle identifier.
2. canonicalMetricTitle collapses digits, and a lyric has none. Adding LyricsX to the existing list would have changed nothing —"I walked through the door" canonicalizes to itself.
So this adds a second title shape rather than a second entry. A metric title has a stable skeleton worth keeping ("CPU #" and "Network #" still tell two iStat items apart); a lyric has none, every character is the volatile part.
For such an owner the whole title collapses to a constant, leaving the instance index as the only distinguishing part. That is sufficient while the owner contributes one item, and is why this stays an allowlist rather than becoming a heuristic.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(controlitem): let divider seeding write through its own guard (#890)
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): prune saved-layout entries that can never match again
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(capture): refresh display topology when no display holds the windows (#794)
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(settings): name the display the layout editor is showing (#886)
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(controlitem): restore divider positions on hide and removal too (#890)
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* build(deps): adopt swift-subprocess 1.0.0 and drop the env trampoline
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): address PR 889 review feedback (#892)
Signed-off-by: VailElla <63608800+VailElla@users.noreply.github.com>
* fix(review): address CodeRabbit findings
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix: reject zero-area intersections when selecting the host display.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: René <diazdesandi@proton.me>
* refactor(settings): retire the experimental move and click toggles
Withdraw the gesture move-drag experiment. It was trialled behind
`useGestureMoveDrag` to remove the warm-up attempt every move needs —
the first press nudges the item a pixel, the second teleports it (#881)
— by pressing on the item, dragging across the bar and releasing at the
target instead of pressing and releasing at the destination with the
item's window ID stamped on the press. It did not remove the warm-up,
so the geometry, `postGestureDragSteps` and the A/B log field are gone
and the warm-up attempt stays an open problem.
Promote AX click delivery to the default. It has been on the
experimental flag long enough without failure reports, and it still
falls back to the synthetic click on any error, so `useAXClickDelivery`
now defaults to on and stops being surfaced in Settings. Both keys are
still read from defaults, so anyone who set them keeps their choice.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): restore the saved layout reliably during startup settling
Folded from 3 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(menubar): carry the saved-layout cooldown bypass to the launch restore
The saved layout was never restored at launch. `skipRecentMoveCheck`
only clears `cacheItemsRegardless`'s own 1 s gate; `applySavedLayout`
keeps a separate 5 s gate, and the launch pass reaches it through the
recache that `relocateNewLeftmostItems` schedules — where the flag was
dropped entirely.
So the restore always ran behind a cooldown this same chain had just
stamped by moving our own control item ~0.3–0.5 s earlier. In the #881
logs the deltas were 0.476, 0.475, 0.434 and 0.438 s: deterministic,
never outside the window. `applySavedLayout` returned false every time
and the cycle then persisted the drifted arrangement over the saved
one.
Add `bypassSavedLayoutCooldown` to `cacheItemsRegardless`, request it
from both launch call sites, propagate it across both nested recaches,
and honour it as `bypassMoveCooldown` in `applySavedLayout`. Nothing
else opts in, so ordinary re-applies keep the 5 s debounce that stops
them stacking up when many apps relaunch at once.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
fix(menubar): relocate the Thaw icon during startup settling
macOS can restore our two control items in the wrong relative order,
parking the visible one left of the hidden divider — off screen. In the
#881 logs the icon sat at itemMinX=-3606 against a targetMinX of -3497.
The correction was deferred for the whole settling period, so the menu
bar had no Thaw icon for as long as settling ran: ~8 s when Control
Center is slow to hand out source PIDs, which reads as the app having
crashed.
Exempt the icon from that deferral. `LayoutSolver.planThawIconMove`
returns the visible control item when it is left of the hidden divider,
sharing the leftmost-item selection with `planLeftmostMove`, and
`relocateThawIcon` is extracted so the settling path and the planner's
`.thawIcon` case reach the same move from different inputs. Acting this
early is safe because the decision turns only on geometry and our own
control item's tag — it is the namespace tags of *other* items that
aren't trustworthy yet.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
feat(menubar): apply the saved layout early for resolved identities
Waiting for every sourcePID to resolve before touching the bar means
the user watches macOS's arrangement for as long as resolution takes —
~8 s on a dense bar in the #881 logs, most of it Control Center being
slow to answer.
Attempt one apply during settling, restricted to identifiers whose live
item already has a resolved sourcePID. That is safe because
`planLCSMoveSequence` intersects current with desired and only moves
identifiers present in both: dropping one from desired leaves that item
untouched rather than mispositioned, and the unrestricted settling-end
pass then moves only the remainder, since LCS keeps whatever is already
in place. The match is exact on `uniqueIdentifier` — a base-identifier
fallback could admit an unresolved sibling of a resolved item
(`Item-0:1` resolved, `Item-0:2` not), which is precisely what this
excludes.
`didAttemptEarlySavedLayoutApply` bounds it to one attempt per settling
period, the majority-unresolved guard is waived for these callers since
running while most PIDs are unresolved is the whole point, and the pass
bails if nothing resolves in time to be worth moving.
This does not address why resolution is slow: Control Center's own
items fail the 1 pt centre-distance check in SourcePIDCache and never
qualify for the relaxed hosted-extras pass. That stays open.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(sourcepid): stop skipping Thaw's own disabled control items
`ControlItem` sets `button.isEnabled = false` for a divider in
`.hideSection` so the spacer stays inert, which is the normal steady
state for a collapsed section. The batch AX walk then skipped it —
`guard AXHelpers.enabledAttribute(child) != false` — so Thaw's own
dividers got no sourcePID for as long as their section stayed
collapsed.
That disabled two of the four `ControlItemPair` strategies. Fallback 2
matches on `sourcePID == ourPID`. Strategy 4's candidate predicate in
`selectViaAXFrame` requires `candidate.isOwnProcess`, which is the same
comparison — so the AX-frame correlation could not confirm a single
candidate no matter how good the geometry was.
In the #899 log both dividers sit in the unresolved list with an exact
positional match already found:
windowID=127922 title=Thaw.ControlItem.Hidden
... in app=com.stonerl.Thaw distance=0.0 closestAXEnabled=false
distance=0.0 clears the 1 pt tolerance the pass requires; only the
enabled check rejected it. `ControlItemPair` then fell through to
"strategy 4 found no confident correlation among 48 candidate item(s)"
and the cache kept its last-known-good pair. It also explains the
"strategies 1 through 3 never fired once" observation in #895:
fallback 2 is dead by construction whenever a section is collapsed.
Exempt our own bundle from the enabled check in the first pass, where
the exact-match resolution happens. The hosted-extras pass is left
alone: it only considers windows whose title indicates the owning
bundle, and `Thaw.ControlItem.Hidden` fails that test against
`com.stonerl.Thaw`, so it never sees our dividers anyway.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(layout): allow drops into badge-only sections (#897)
Signed-off-by: Alvie Stoddard <alviestoddard@gmail.com>
* fix(controlitem): stop resetting the hidden divider on every preflight
`preflightSetup` seeded the hidden divider's preferred position when none
was stored, then unconditionally re-stamped it to 1 a few lines later.
The second write ignored whatever the user had.
It was inert for six months. ff7517f7 added a subscript guard that
refuses divider position writes, so the reset never reached defaults.
a1e566d4 (#890) routed divider seeding around that guard to fix the
first block, which woke the second one up as well.
`StatusItemStorage.init` calls preflight, so this ran on every launch
and every `recreateStatusItem()`. On a populated bar the reporter
measured Hidden going from 1051 to 193 against a Visible at 160, and
the save that followed persisted the collapsed span, draining
savedSectionOrder from 64/46/12 to 42/52/12 across two clean launches
(#895, seeded by #890).
Drop the reset. The seed above it already covers the only case that
needs a default, and `resetChevronPositions()` keeps the deliberate,
user-triggered reset. `preflightSetup` now takes an `Identifier` rather
than a `ControlItem` so the three added tests can call it directly;
nothing else in it read the control item.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): let the early saved-layout apply past the move cooldown
The early apply added in c48c1512 never ran. `relocateThawIcon` moves our
own control item within ~100 ms of launch, which stamps the 5 s move
cooldown, and no settling-period call site sets
`bypassSavedLayoutCooldown`, so every poll that reached the early apply
was inside a window that same launch had just created.
In the #881 log for build fec231c the apply was rejected at 16:56:19.457
for a cooldown stamped at 16:56:16.375. The reporter therefore still
watched macOS's arrangement for the whole 5.2 s settling period and then
the entire reorder as twelve visible sequential moves, which is the
"icons disappear, all reappear, then disappear one by one" they
described.
Bypass the cooldown here rather than inheriting it. This block only runs
inside `guard !isInStartupSettling`, and the one-shot flag bounds it to a
single attempt, so the cascading re-applies the cooldown guards against
cannot happen.
Also set `didAttemptEarlySavedLayoutApply` only when the apply actually
dispatched. It was set before the call, so an apply rejected by any guard
consumed the single allowed attempt and no later poll retried.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): verify item moves by adjacency, not coordinate equality
`itemHasCorrectPosition` re-read the item's bounds and the target's
bounds independently and compared them for exact `CGFloat` equality. That
cannot succeed on a bar that reflows: our own drag displaces the target
too, so the item lands where the target *was* and is then compared
against where the target now is.
In the #881 log for build fec231c, one H_ctrl move:
attempt 1 targetMinX=-4222.0 itemMinX=-3526.0 -> landed (-4222.0, 0.0)
attempt 2 targetMinX= 794.0 itemMinX=-4222.0 -> landed (832.0, 0.0)
attempt 3 targetMinX= 794.0 itemMinX=-4222.0 (identical to attempt 2)
The target's measured minX swung by ~5000 pt between attempts while the
item sat still, and all eight attempts were spent re-dragging against a
destination that had already moved. `postMoveEvents` only returns without
throwing once both waits observed an origin change, so those were eight
real drags, which is the mechanism behind the walking layout in #900.
Ask for the ordinal relationship instead. One enumeration, sorted by
minX, then an index comparison: both operands come from the same
snapshot and cannot drift apart mid-check. It also sidesteps
`getCurrentBounds` mixing coordinate spaces, since its windowID path
answers for parked offscreen windows while its tag-matching fallback
answers from the on-screen list, and which one runs depends on timing.
`.activeSpace` rather than `.onScreen` because an item moved into a
collapsed section is parked offscreen and that landing still has to be
confirmable. Source PIDs are left unresolved: this runs once per attempt
and only tags, window IDs and bounds are needed.
Add `EventError.staleDestination` for the second half. When the target
has travelled further than the display's width since the move was
planned, the plan describes an arrangement the bar no longer has;
retrying re-plans against fresh geometry each time and leaves a
different partial arrangement behind on every pass. Throw instead, and
short-circuit it in the catch so it is not retried. No failure is filed
against the item's owner, which did nothing wrong.
The threshold is extracted as `destinationIsStale` and covered by
`StaleDestinationGateTests`: the display's width sits well above the
item-width nudge a successful landing causes, while still catching a
target that has crossed into another section or coordinate space.
The adjacency check itself stays uncovered for the reason
`EarlySavedLayoutRestrictionTests` already documents: it needs a live
`appState` and real Window Server items.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(bridging): reject zero-area display intersections when cropping
`CGRect.intersection` only returns the null rect when two rects are fully
disjoint. Rects that merely touch along an edge intersect to a zero-width
or zero-height rect, which reports `isNull == false`, so the guard let a
stale `screenBounds` through as though it still overlapped the window
union and the crop was computed against the wrong display.
`isEmpty` covers the null rect and the zero-area ones together.
The SwiftLint suppression is for `is_disjoint`, which fires on the
`intersection(_:).isEmpty` shape but is about `Set`; `CGRect` has no
`isDisjoint(with:)`.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(layout): withhold a partial arrangement from the saved order
The third item in #900: a bulk apply that gives up partway leaves the bar
in an arrangement nobody chose, and the next cache cycle wrote it to
`savedSectionOrder` as though the user had made it. That destroys the
order the batch was trying to restore, so the following pass plans
against the partial result and moves things somewhere new again. The bar
walks instead of converging, which is what makes the failure look like
Thaw shuffling icons on its own rather than one move that didn't take.
Count the moves Phase 6 planned for an item still on the bar and then
did not enact: the H_ctrl and AH_ctrl failures, the per-item
cross-section fallbacks, the LCS loop's own failures, its backoff skips,
and the two mid-sequence abandonments (losing the dividers after Phase 1,
and losing them inside the loop, which forfeits the whole remainder).
A non-zero count arms `unfinishedMoveBatchObservedAt`, a clean batch
clears it, and `shouldPersistSavedOrder` reads it as
`hasUnfinishedMoveBatch`.
Cancellation is deliberately not counted. A cancelled sequence is one a
newer apply replaced, and that apply arms or clears the gate itself.
The arm expires after 30 s rather than holding until an apply finally
comes back clean. A move can fail on an owner that never responds, and
the failure ledger's backoff eventually stops retrying it altogether;
without a bound, one such item would freeze the saved layout for the rest
of the session and a rearrangement the user made by hand would never be
recorded. The window matches `confirmedDivergence`'s, which is what the
retry apply needs before it will dispatch.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(appearance): separate the shape border for the menu bar and the Thaw Bar
The border row in the menu bar appearance editor now has a checkbox per
place: one for the menu bar overlay, one for the Thaw Bar. A contrast
border on the Thaw Bar makes the hidden section easier to spot over a
maximized app, which is distracting on the menu bar itself.
`hasBorder` on the partial configuration becomes a computed facade over
the two new flags, so the Ice import and the colour and width rows keep
working as they did. Settings saved before the split carry only
`hasBorder` and seed both flags on decode; encode still writes it so a
border stays visible on a build that predates the split.
The overlay panels are no longer created for a border that only applies
to the Thaw Bar, and the menu bar checkbox is disabled when the shape is
None, since there is no shape to trace.
Closes #248
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(spacing): apply the profile's spacing offset instead of a stale one
`MenuBarItemSpacingManager.offset` starts at 0 on every launch and is only
written by `applyActiveDisplaySpacing`, which runs on a real display
transition or from `configurations.didSet` — and that observer returns
early on `oldValue != configurations`. Applying a profile whose display
configurations already match the live ones, which is the usual case since
the profile is where they came from, therefore left the offset at 0.
`applyProfile` then called `applyOffset()` with it, wrote the system
default over the user's spacing, and relaunched every menu bar app to do
it.
`applyProfile` now takes the offset from the configuration the snapshot
just installed, and setup seeds the spacing manager from the active
display so nothing else reads a value the machine isn't running. The
notch overflow budget in `MenuBarItemManager` reads the same property and
was mis-measuring by the same difference.
Both, along with `applyActiveDisplaySpacing`, now go through
`activeDisplaySpacingOffset` so they can't disagree.
Closes #903
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(displays): show only the always-hidden section in the Thaw Bar
A new per-display setting, "Always-hidden items only", routes the
always-hidden section to the Thaw Bar while the hidden section keeps
expanding in the menu bar. Reaching the always-hidden items inline
always unfurled the hidden section along with them, since they sit to
the left of the hidden control item, which is the whole problem for
people who keep a full menu bar hidden and only need the rarely-used
items now and then.
The routing rule moves into `MenuBarSection.usesThawBar(for:...)`, a
pure static beside `forcesIceBarForNotchOverflow`, so the truth table
is testable without a live menu bar. The display-wide setting still
wins over it, and notch overflow can still force the bar on top of
both.
The application-menu sink now checks which section the panel is
showing: a section presenting in the panel expands nothing inline, so
there is nothing for the menus to make room for.
The setting is available over the Settings URI as
`useThawBarForAlwaysHidden`, scoped like `alwaysShowHiddenItems` to
displays that are not already routing everything to the bar, and it is
carried in profiles and the global template like every other
per-display field. Configurations saved before this decode it as off.
Closes #751
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(search): add a toggle to move the pointer to revealed items (#769)
Opening a menu bar item from the search panel left the pointer wherever
it happened to be, so the menu the item opened was nowhere near it and
had to be reached with a deliberate mouse trip across the screen.
Add an Advanced > Menu Bar Search toggle, off by default, that warps the
pointer onto the item once it has been revealed. The point is read from
the item's live window bounds rather than its cached ones, since those
predate the reveal, and it is dropped when it lands on no display:
CGWarpMouseCursorPosition clamps such a point to a display's leftmost
edge, which sits under the Apple menu.
Closes #769
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): bound automatic re-applies when move batches keep failing
On a bar that refuses synthetic drags (#900), withholding a partial
arrangement from the saved order keeps the divergence alive, and the
divergence re-dispatches the next bulk apply: an unbounded loop that
hides the cursor for the length of a batch on every pass while landing
a different partial arrangement each time (#899). Three bounds:
- An unfinished batch earns one automatic retry; after a second
consecutive unfinished batch, dispatch is rationed to one attempt
per 60 s cooldown. User-initiated applies are exempt but still feed
the streak.
- A batch abandons its remaining moves after three consecutive
failures, instead of burning the full attempt budget on every queued
item with the cursor hidden throughout.
- Notch overflow ejections now go through the failure ledger, so a
candidate whose eject keeps failing backs off instead of being
re-dragged on every windowID change.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* test(settings): cover the useThawBarForAlwaysHidden scope paths
The specific-display arms were tested; the scope-wide ones were not.
Covers the allNonIceBar value and toggle broadcasts and the
not-implemented scope arm in DisplaySettingsManager, the named-display
set arm and default-scope set/toggle routing in SettingsURIHandler,
the invalid-boolean refusals on both, and the typed per-display
lookup. Still uncovered on this branch: the spacing-offset seed and
the applyProfile offset push, both of which sit behind a live
AppState, which the unit suites deliberately never construct.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): stop misreading windows as open menus
Folded from 3 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(menubar): never count a display-sized window as an open menu
Drop-shelf utilities (Droppy) raise an invisible menu-level drag-catcher
over the whole screen during any drag session — including the user's own
drag inside the layout bar. A window that spans the display contains the
pointer wherever it goes, so the probe's under-pointer rule reported an
open menu for as long as the overlay stayed up, and every layout bar
drag waited five seconds and then deferred itself: items snapped back,
looked stuck, and only a tab switch refreshed them (#899 log,
2026-08-07). In that log, 85 of 89 probes flagged the same Droppy
window, including the very first probe of the session, which the
grandfathering should have ignored — only the pointer rule can override
it, which is how the overlay was identified from the log alone.
A candidate covering at least half of a display it touches is now
classified as furniture regardless of age or pointer, and the probe's
diagnostics log candidate bounds so the next report can confirm the
geometry directly.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
fix(layout): trust a completed drop over the open-menu probe
Correction to the previous commit's reasoning: Droppy is a notch app,
so its untitled status-level window sits at the top of the screen and
is not necessarily display-sized — the size gate may never fire for
it. What the #899 log proves regardless is that the probe held every
layout bar drag hostage for the window's whole lifetime.
The stronger invariant does not depend on the window's geometry: menu
tracking captures the event loop, so a drag cannot complete inside the
settings window while any menu bar menu is open. A move requested by a
drop in our own layout bar therefore skips the menu-open wait instead
of stalling five seconds and silently discarding the drag. Automatic
applies keep the guard; the size gate stays as defense against
genuinely display-sized overlays.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
Revert "fix(layout): trust a completed drop over the open-menu probe"
This reverts commit e45105d3781b7d8f0998344e41ebb14582ded879.
The completed-drop inference does not hold across processes. The wait
this bypassed is what keeps a synthetic Cmd-drag from tearing down an
open Control Center interaction (Wi-Fi picker, input-method panels),
and those do not reliably dismiss when the user clicks or drags in
Thaw's own settings window. The Droppy false positive stays addressed
by 07e92397's display-size gate, and the probe now logs candidate
bounds, so the next diagnostic log will show whether that gate covers
Droppy's window or the classifier needs a different discriminator.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(profiles): mark a profile active after updating it from current state
An "Update All" captures the running state into the profile, so the
profile is the configuration in effect by construction — but nothing
set activeProfileID, so the checkmark stayed wherever it was and the
updated profile read as "not applied" (#904). Marking it active also
lets the existing re-arm gate refresh the in-memory layout spec for a
profile that was not active before the update; the captured layout
matches the live bar, so the re-arm moves nothing.
Scoped to full updates: a layout-only or configuration-only update
leaves the other half of the profile potentially different from the
running state, and claiming "applied" for it would be a lie.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): diagnose silent move refusals and cover the new code
Folded from 2 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(menubar): diagnose silent move refusals and failures
Two reports could not be traced because the failure paths said nothing:
#905: a refused move emitted no log line and a generic alert. isMovable
is now defined through a named ImmovabilityReason, both refusal sites
(the layout editor's disabled-item alert and move() itself) log the
resolved identifier and the gate the decision was made on, and the
alert copy stops blaming macOS for Thaw's own unresolved-Control-Center
safety gate. For a degraded identity, a one-shot AX correlation names
the hosted slot's real owner in the log and the alert, instead of the
"Menu Bar Item" fallback the decision was never made on. Per plan 014,
AX identity stays display-and-diagnostics-only — it does not re-tag the
item or feed matching.
#900: some twenty sites throw a bare cannotComplete; the profile-layout
log was a wall of them with no way to tell a cancelled superseded apply
from a dead owner. Every silent site in the move path now logs its
stage and the underlying error it used to swallow. One behavioral fix
falls out: a move that was in flight when a newer apply cancelled its
batch was recorded as the item's failure, earning innocent items
backoff windows and re-arming the save withhold for a tally the newer
apply owns. The Phase 6 catch now applies the same rule as its loop
head and leaves a cancelled move unrecorded.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
test(menubar): cover the branch's remaining reachable new code
Three additions close every uncovered new line that the unit suites
can reach: the gate log descriptions #905's refusal sites emit, the
open-menu size rule's empty-bounds guard, and prunedSectionOrder's
preservation of a section key outside the known three. What remains
uncovered on the branch is @MainActor runtime code (moves, applies,
event posting, SwiftUI panes) that the suites' no-AppState pattern
deliberately does not construct.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): let an AX-correlated identity promote an unresolved Control Center placeholder
#905 part 2: the identity-preference fallback. When the Layout editor
captures a generic Control Center slot (com.apple.controlcenter:Item-N)
the source-PID cache has not resolved this cycle, the drag is refused by
the unresolvedControlCenterPlaceholder gate. The catalog already holds an
app-owned form (e.g. at.obdev.littlesnitch.agent:Item-0) for the same
physical item, but the two identifiers are distinct keys, so the saved
layout never matches.
Before refusing the drag, run the same bounded AX correlation
logMoveRefusal() already uses. On a confident hit, build a synthetic
MenuBarItem re-tagged under the owning app's bundle ID namespace and
carrying the owner's PID as sourcePID (UnresolvedPlaceholderAlias):
- isMovable becomes true for the alias, so move(...)'s inner guard
let synthetic events through; AppKit repositions the slot by windowID.
- savedSectionOrder eventually writes the app-owned identifier once the
source-PID cache catches up (or AppKit's own autosave holds position
meanwhile).
- windowID/bounds/ownerPID/title/isOnScreen are preserved; only the tag
namespace and sourcePID change, so the live slot remains the target.
The pure halves (appBundleID from:excluding:thawBundleID, and
aliasedItem for:appBundleID:hostPID) are nonisolated and unit-tested:
identifier-preference, host/Thaw exclusion, title/help fallback,
non-placeholder rejection, static-system-item rejection, tag re-tagging
with identity preservation, movability/persistability, and that
uniqueIdentifier matches the saved-layout app-owned key.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): exclude parked items from the H_ctrl drag anchor (#881)
At launch the hidden section's items are parked thousands of points left
of the display by the control item's collapse. planHiddenDividerAnchor
picked the first desired-hidden item (e.g. lmstudio at minX=-4222) as
the H_ctrl drag anchor — a parked item. The drag failed all 8 retries:
each attempt briefly brought the divider on-screen (icons
disappeared/reappeared), then AppKit snapped it back to the parked zone
on mouse-up, producing the cursor seizure and icon storm reported in
#881. The per-item LCS pass, which runs after the H_ctrl move, DID
succeed at repositioning items one-by-one, but only after the 1-second
futile H_ctrl retry cycle.
Fix: exclude items whose center falls on no screen from the anchor
candidate set (LayoutSolver.isOnScreen). When all desired-hidden movables
are parked (the launch case), the planner returns nil, the H_ctrl
boundary move is skipped, and the per-item LCS pass runs immediately
instead of after a storm.
The on-screen check reuses the center-on-screen convention from
itemsSpanMultipleDisplays: an item parked at negative X does not fall on
any screen frame and is excluded.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): stop H_ctrl boundary-move storms and gate the late-arrival re-sort
Folded from 3 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(test): make UnresolvedPlaceholderAliasTests compile
The suite added in 162242d6 never built, so the whole ThawTests target
failed to compile:
- appBundleIDRejectsHosts carried `arguments:` as a function parameter
label rather than a @Test argument, which is a parse error. Moved it
into the macro and named the parameter it binds.
- Three cases declared `let alias` twice — once for the optional result,
once for `try #require(alias)` — in functions that were not `throws`.
Collapsed each into a single `try #require(...)` and marked the
functions `throws`.
No assertion changes; every case tests what it did before.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
fix(menubar): stop the H_ctrl boundary move storming on a parked divider
#881 excluded parked items from the H_ctrl drag anchor. #899 is the same
drag failing from the other side: the anchor is back on the bar and passes
the anchor filter, but the divider itself is still parked at minX=-3950.
The drag point is on screen so the owner accepts the events, then AppKit
snaps the divider home on mouse-up and every attempt reports "events
succeeded but item not at destination".
The reporter's log runs 22 seconds without converging. Profile re-sort
fires seven times, the hidden section alternates 4 items -> 0 -> 4, and
hiddenBoundaryMismatch alternates 5 -> 9 -> 5. Each pass burns the full
eight-attempt budget on a drag that cannot land, then the per-item pass
flips the membership back so the next re-sort has identical work. 246
events went to the session event tap in those 22 seconds, warping the
pointer between x=1074 and x=-3926; the log ends because the reporter
killed the app.
Two guards, both ahead of any event posting:
- Skip the boundary move when H_ctrl itself is not on screen. The drag
cannot land, and the per-item LCS pass repositions items without needing
the divider to travel.
- Put the boundary move under the same failure ledger the per-item moves
already use: skip while under backoff, record success and failure, and
leave a move cancelled by a newer apply unrecorded. Previously nothing
bounded this move, so any persistent failure was retried in full by
every re-sort.
Both skips count toward unenactedMoveCount, so a bar whose boundary was
never fixed is still withheld from the saved order.
Adds MenuBarItemFailureLedger.isUnderBackoff(for:), which derives the key
the same way recordFailure does. The key-taking overload answers for the
raw uniqueIdentifier, which only matches for owners that do not retitle
themselves.
The guard itself sits in applyProfileLayout, which needs a live AppState
and WindowServer, so the tests pin the predicate and the ledger rather
than the branch: ParkedDividerLog carries both shapes from the field log
and ParkedDividerTests locks that the anchor filter passes the state the
new guard has to catch.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
fix(menubar): gate the late-arrival re-sort on the unfinished-batch streak
automaticBulkApplyPermitted had one caller, applySavedLayout. The other
automatic dispatch — scheduleProfileResort, for late-arriving items — fed
the streak through recordBulkApplyOutcome but never read it, so a bar
whose batches keep failing re-sorted on every late arrival with nothing
bounding it.
That is the path #899 stormed down: seven passes in 22 seconds, each a
full move batch with the cursor hijacked, while applySavedLayout sat
correctly gated on its own cooldown. With the gate wired in, the same
sequence stops after two unfinished batches and waits out the 60s
cooldown.
User-initiated applies still bypass it. applyProfile calls
applyProfileLayout directly and never routes through scheduleProfileResort,
which preserves the existing rule that an explicit request is worth a
fresh attempt regardless of history.
The gate function's own behaviour is already covered by
AutomaticBulkApplyGateTests; this commit only adds the second caller.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(cli): add --reset-layout for repairing a bar before launch
A parked divider persists as an NSStatusItem Preferred Position, so a bar
wrecked badly enough comes back wrecked on the next launch and starts
moving items — and the pointer — before the user can reach the Settings
pane's reset. #899's reporter had to kill Thaw from a terminal; this lets
them repair it from the same terminal, before starting it again:
/Applications/Thaw.app/Contents/MacOS/Thaw --reset-layout
LayoutResetCommand is the persisted half of resetLayoutToFreshState():
it clears the saved order, the known-item set, the pinning sets and the
relocation bookkeeping, then re-seeds the visible and hidden divider
positions. The divider half matches resetLayoutToFreshState() exactly,
including leaving the always-hidden divider alone — it is placed
dynamically and has no seed value to restore. Only UserDefaults is
touched, so it needs no AppState and runs with the app not running.
IceApp loses @main to a ThawMain wrapper. It cannot serve this itself:
its @NSApplicationDelegateAdaptor builds the AppDelegate, and with it the
whole AppState, as the App value is constructed, which is already too
late to decline to run.
Not verified end to end: exercising the flag writes to the real
com.stonerl.Thaw domain, which would reset a live menu bar layout. The
reset logic is tested against a throwaway defaults suite instead, leaving
the two-line entry-point wiring covered only by compilation.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(menubar): ignore unresolved items when detecting late arrivals
uniqueIdentifier is derived from sourcePID via the tag's namespace, so an
item whose PID did not resolve carries a fallback identity: it collapses
into the Control Center host namespace, or repeats its bundle ID as the
title. The late-arrival detector matched on those identifiers without
checking whether they meant anything, so a resolution flap read as a bar
full of new items.
#881's 547c9ba log: 34 items with 16-17 sourcePIDs unresolved, sampled
twelve times over an hour. That is under majorityOfSourcePIDsUnresolved's
strict-majority bar -- 17 of 34 is a dead-even split and does not trip it
-- so every apply ran. The same item alternated between
eu.exelban.Stats:CPU_bar_chart and eu.exelban.Stats:eu.exelban.Stats:1,
and whichever form the previous sort had not recorded read as an arrival.
Twenty re-sorts in 63 minutes, 12 of them inside one minute.
Neither existing guard covers this. The boundary-move guard does not
apply: all 14 boundary moves in that log landed. The unfinished-batch
gate does not fire either: 15 of 16 batches completed, so the streak
reached 1 once and the gate needs 2. This loop is made of successful
applies undoing each other, not of failures.
Excluding unresolved items costs nothing real. A late arrival is an app's
item appearing after launch, and those resolve; the items that
legitimately hold a nil PID (Wi-Fi, Clock, BentoBox) are always-present
system items that never arrive late. The same filter already guards
knownItemIdentifiers during startup settling, where the comment describes
this exact cascade.
The decision moves to a pure lateArrivingProfileIdentifiers so it is
testable outside the cache actor; the tests use the resolved/fallback
identity pairs from the log and fail without the filter.
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(profiles): prune identifiers that can never match a live item
MenuBarItemManager prunes the saved section order every time it loads it
(#788, #815), but nothing ever repaired a profile. A profile is captured
from the live bar, so a capture taken while source-PID resolution was
degraded bakes in provisional identifiers permanently -- and unlike the
saved order, no background pass rewrites it.
#881's reporter carried a profile holding four Control-Center-hosted
entries with no title at all: com.apple.controlcenter: through ::3.
uniqueIdentifier is namespace:title, and unlike MenuBarItemTag.description
it does not omit an empty title, so an item whose title could not be read
persists in that shape. Every apply planned against them.
Two changes:
- prunedSectionOrder drops Control-Center-hosted entries with no title.
The only live item such an entry could match is one whose title was
equally unreadable, and two of those are indistinguishable apart from an
instance index assigned in arrival order. Scoped to the Control Center
namespace: under a real owner an empty title still feeds
planLeftmostRelocation's namespace fallback.
- Profile.resolvedItemOrder prunes on the way out, covering both the
current itemOrder and the legacy savedSectionOrder representation. Every
consumer reads the layout through it or through resolvedItemSectionMap,
including the identifier set arrival detection matches against, so one
prune at the read covers them all.
Order is preserved; entries are dropped, never rearranged (#885).
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* feat(profiles): preview a saved profile's layout from its list row
Adds an eye button per profile opening a popover with the saved item
order per section (app icons resolved from bundle IDs, +N overflow)
and the profile's key behavior settings (#887).
Signed-off-by: René Jiménez <diazdesandi@proton.me>
* fix(ui): round averageColor alpha threshold up to honor the >= contract (#917)
`CGImage.averageColor(using:alphaThreshold:option:)` documents that a
pixel contributes when its alpha component is greater than or equal to
`alphaThreshold`, but it converted the normalised [0, 1] threshold to an
8-bit comparison value with `.rounded(.toNearestOrAwayFromZero)`. For any
threshold whose `255 * threshold` has a fractional part below one half,
rounding to nearest lands one byte below the correct value, so a pixel
whose alpha is just under the threshold was still counted. Concretely, a
pixel with alpha byte 85 (0.3333) leaked in at `alphaThreshold: 0.334`,
yielding a spurious non-nil average instead of nil.
Round `.up` (ceiling) instead: that is the smallest byte satisfying
`byte >= 255 * threshold`, which exactly matches the documented contract.
The default threshold (0.5) and the previously tested values (0, 0.5,
0.9) are unaffected — they sit on or above the .5 boundary where round
and ceil already agree.
Adds a regression test pinning the boundary behaviour (red on the old
round-to-nearest code, green with the ceiling).
Signed-off-by: Yurii Chukhlib <yurii.chukhlib@viber.com>
* fix(menubar): preserve order of rightOfAnchor items sharing one anchor (#919)
LayoutReconciler.applyUnmanagedPlacementsToDesired inserts a .rightOfAnchor
item at firstIndex(anchor) + 1. Inserting after the anchor does not shift it,
so firstIndex(anchor) resolves to the same index on every iteration and each
item in a shared-anchor group lands in the same slot, reversing the group's
relative order. The mirror .leftOfAnchor branch is correct (inserting before
the anchor shifts it, advancing the resolved index), and Pass 3 documents
that unmanaged items must keep their unmanagedUIDs order — so rightOf is the
outlier.
LayoutSolver.planUnmanagedPlacement assigns every unmanaged item that lacks a
saved position the same NewItemsPlacement anchor, so several items sharing one
rightOfAnchor anchor is the common case (e.g. two new menubar apps appearing
in one profile apply), not an edge case.
Track how many items have already been placed right of each anchor and advance
the insert position past them, so rightOf preserves order like leftOf does.
The existing section clamp and the leftOf/sectionDefault paths are unchanged.
Adds three regression tests: rightOf pair, rightOf triple, and a leftOf mirror
that pins the already-correct path.
Signed-off-by: Yurii Chukhlib <yurii.chukhlib@viber.com>
* feat(menubar): add reliability gates and name Little Snitch after its app
Derived from the diagnostic logs on #899. Everything here is inert by
default except the Little Snitch rename.
- bulkApplyIdleThresholdMs / bulkApplyIdleWaitCapMs gate *automatic*
bulk applies on a lull in user input. waitForUserToPauseInput already
gates each move; this gates the batch, which holds the cursor hidden
for its whole length, so one dispatched the moment a late arrival is
noticed can take the pointer away mid-interaction and then contest it
move by move (#723, #899). It defers and never cancels: the cap
guarantees the batch still runs, because a saved layout that is never
restored is a worse failure than one restored during input. Off by
default.
- enforceConcealedSectionOrder, set false, surrenders ordering *within*
the hidden and always-hidden sections while keeping membership. A move
costs the same whether or not its result can be seen, and those
sections are revealed through the Thaw Bar, which renders from the
cache. Implemented by rewriting the desired sequence before planning
so the LCS sees those items as already in place; filtering planned
moves afterwards would strand survivors against anchors the plan
assumed had shifted. On by default.
- automaticArrangementEnabled is the blunt escape hatch: false stands
down the late-arrival re-sort and the saved-layout restore while
leaving profile applies working, so a user keeps a deliberate way to
arrange. Surfaced under Advanced layout controls, reusing catalog
strings that already carry all 19 localizations. On by default.
- Little Snitch's status item is owned by at.obdev.littlesnitch.agent, a
helper nested inside the app bundle, so it was named after a process
the user never installed. canonicalBundleID folds it into
at.obdev.littlesnitch. That changes uniqueIdentifier, so
canonicalIdentifier migrates saved orders and profile data on read —
before pruning, or the renamed entry is discarded as unmatchable
first. The #899 log confirms real profiles carry the agent form.
OneDrive was investigated and deliberately excluded: com.microsoft
.OneDrive-mac is the installed app's own bundle identifier rather than a
helper's, so folding it into com.microsoft.OneDrive would rename a real
app to an identifier no process reports and orphan its saved position. A
regression test pins that OneDrive identifiers are left alone.
Signed-off-by: Rene <diazdesandi@proton.me>
* fix(profiles): stop profiles from restoring the diagnostic logging switch
enableDiagnosticLogging was part of AdvancedSettingsSnapshot, so applying
a profile restored whatever the switch had been when that profile was
saved — off, for every profile that already exists. A user who turned
logging on to capture a profile switch had it switched off by the very
apply they were trying to capture, and the toggle appeared to flip
itself back (#899).
It is a diagnostic control, not a preference: it belongs to the session,
not to a layout. Removed from the property list, capture, apply,
CodingKeys, the memberwise init, init(from:) and the defaults factory.
Safe in both directions without a version bump. Every property in the
snapshot decodes with decodeIfPresent and a Defaults.DefaultValue
fallback, so a current build ignores the key still present in profiles
written earlier, and an older build reading a profile written now falls
back to its own default rather than failing to decode. Both directions
are pinned by tests, including one asserting the key is no longer
encoded at all.
Untouched where it should still work: the Advanced pane toggle, the
thaw:// settings URI, and Reset all settings.
Signed-off-by: Rene <diazdesandi@proton.me>
* fix(menubar): resolve Thaw's own control items from their own windows
Folded from 3 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(menubar): never resolve the visible control item as a section divider
ControlItemPair's last strategy correlates Thaw's own AX element frames
against candidate CG windows. It already filtered the visible control
item out of the frames being matched *against*, with a comment saying it
must never "be returned as the hidden or always-hidden control item" —
but nothing excluded it from the windows that can be *selected*.
The visible control item is own-process, so it passes the isOwnProcess
test on frame alone. When the hidden divider is absent from the
candidate list — parked far offscreen, or dropped by the active-space
filter — it can be the only own-process candidate left, and the hidden
AX frame correlates onto it.
Observed on three reporters' logs, all on 54345d40:
cacheItemsRegardless: found control items, hidden windowID=120, alwaysHidden=122
ControlItemPair: strategy 4 (AX frame) matched hidden control item
via AX-frame correlation (windowID=118)
where 118 is Thaw.ControlItem.Visible. Fourteen times in one session,
28 in another, six in a third, and in every case with no always-hidden
partner resolved.
Downstream, section boundaries are measured from the wrong window: the
hidden section reads as zero width (hidden.minX == alwaysHidden.maxX,
drifting left one item width per pass), so saveSectionOrder skips and —
since c3317dfd, which removed the accidental recovery along with the
corruption it was fixing — applySavedLayout refuses too. Nothing
persists, and every item lands visible after a restart. That is #927;
the same misreading leaves no region left of the notch for hidden icons,
which is #924, and no divider to verify drags against, which is #923.
Matching on title rather than window ID because title survives the
identity degradation that reaches this strategy at all: it comes off the
CG window, not from sourcePID, whose failure is what defeats the three
earlier strategies.
Refusing to match is the correct outcome here. The caller logs "missing
control items" and bails, which is recoverable; returning the wrong
window is not. This does not restore resolution when sourcePID is
unavailable — Thaw still cannot identify its own control items from its
own state — which is the larger fix and wants its own issue.
Refs: #923, #924, #927, #905
Signed-off-by: Rene <diazdesandi@proton.me>
fix(menubar): identify Thaw's own control items from their own windows
Every route to a control item went through the enumerated item list, and
each one fails in the same conditions:
- the primary lookup needs the window to be present in that list, and it
drops out when parked far offscreen or filtered off the active space;
- tag matching needs an intact namespace;
- title matching needs a resolved sourcePID.
The last two depend on identity channels that degrade together whenever
the item service's PID resolution fails — which is the same failure that
strands the window. What was left was frame correlation guessing at
Thaw's own dividers, and 9159547f only stopped it guessing *wrong*; the
pair still went unresolved.
None of that indirection is necessary. Thaw creates these NSStatusItems
and holds their windows, so their IDs are known first-hand.
MenuBarItem.ownControlItem(windowID:) asks the window server about one
specific window rather than searching a list, and stamps our own PID so
the namespace resolves to Thaw even when nothing else about the item's
identity does. ControlItemPair consults it after the primary lookup and
before the heuristics.
Deliberately narrow, per shouldRecoverOwnControlItem: only with an
authoritative window ID in hand, and only when that window is absent
from the list. Present means the primary lookup already claimed it, and
rebuilding would hand back an item the caller expects to have been
removed from `items`. A window the server no longer knows returns nil
rather than a stale item dressed up as a live one.
The always-hidden partner gets the same treatment, though a nil partner
stays legitimate — that section can be disabled.
This is the capability the macOS 27 branch worked around by persisting
its own items. Frame correlation stays as the last resort for the case
it was written for: no authoritative ID at all, early in startup before
the status items exist.
Refs: #923, #924, #927, #905
Signed-off-by: Rene <diazdesandi@proton.me>
feat(menubar): make control-item resolution legible in the logs
Two gaps made the #923/#924/#927 diagnosis slower than it should have
been, and would have made the next one just as slow.
The three "hidden section has zero width between the dividers" warnings
printed coordinates but no window IDs, so a genuine collapse and a
misidentified pair produced identical output. They were in fact the
latter — the visible control item was being returned as the hidden
divider — but nothing in the warning could show that. All three now name
the windows they measured.
Only the last resolution strategy logged on success, so a log showing
"strategy 4" told you that strategy fired but never that the earlier
three had been tried, and a log showing nothing told you nothing at all.
Each path now says which one resolved the pair, and total failure is a
warning rather than a silent nil.
All debug-level except the failure, so the cost is a few lines per cache
cycle in a diagnostic log that already carries hundreds.
Refs: #923, #924, #927, #905
Signed-off-by: Rene <diazdesandi@proton.me>
* fix(onboarding): show the onboarding permissions screen when access is missing
The permissions window rendered two different screens. First launch got
the glass onboarding flow ending in ThawPermissionsView; every later
launch with missing permissions got PermissionsView, a 760x600 layout
predating the redesign. A user whose permissions were revoked met a
screen they had never seen before, at a different size, with different
cards and typography.
Signed-off-by: Rene <diazdesandi@proton.me>
* feat(menubar): add a flag to post move events to the window's owner
Thaw aims synthetic move events at the app whose status item it is:
getEventPID = item.sourcePID ?? item.ownerPID
That was right before macOS 26, when the owning app also owned the
window. On 26 Control Center hosts every status item window, so the CG
owner of the window being dragged is Control Center while sourcePID
names a process that owns no window in the drag at all.
Posting to the host instead is what the window server appears to expect.
Control Center creates the drag's replicant scene and then delivers
NSStatusItemStartDragAction to the owning app itself, so the host is the
correct recipient and the owner's identity is never needed to perform
the move.
postMoveEventsToWindowOwner makes ownerPID the target unconditionally,
and with it on, unresolvedControlCenterPlaceholder stops making an item
immovable. That gate exists because "posting drag events to Control
Center for a placeholder times out" — a statement about events aimed at
an owning app that did not exist. Addressing the host removes the need
to know the owner, so the premise is what this flag puts under test.
Two things to learn from a live run:
- whether an item whose owner never resolves (#709, #905 — observed live
with every AX field nil and no marker window) becomes movable;
- whether itemResponseTimeout and "Move events failed, posting fallback"
are the same wrong-target bug (#900, #923, and the notch-overflow
ejections in #924, where com.apple.TextInputMenuAgent:Item-0 had a
resolved sourcePID and still could not be dragged).
Off by default; this redirects every synthetic event.
Refs: #709, #900, #905, #923, #924
Signed-off-by: Rene <diazdesandi@proton.me>
* fix(menubar): keep immovabilityReason out of user defaults
2bc73ad0 read postMoveEventsToWindowOwner inside immovabilityReason,
which made a pure derivation of the item depend on process-wide state.
The test target reads the same com.stonerl.Thaw domain, so enabling the
flag on a development machine failed eight tests that assert the default
behaviour — ImmovabilityReasonTests, TagTests, CapabilityFlags and the
UnresolvedPlaceholderAlias suite. The suite was answering differently
depending on whose Mac ran it.
immovabilityReason is pure again. The flag moves to
isMovableAddressingWindowOwner, which lifts only
unresolvedControlCenterPlaceholder — a prohibited system item stays
prohibited wherever its events go — and is read at the two gates that
actually dispatch a move: the layout panel's isEnabled and the
manager's move guard.
Signed-off-by: Rene <diazdesandi@proton.me>
* fix(i18n): use the translated empty-section string in the profile preview
The profile preview shipped "No items", a new catalog key with zero
localizations, while "No items in this section" was already present with
all 19. Same meaning in this context — it labels a section of the
preview that has no items — so the existing key is used instead and the
string is translated everywhere.
Slightly longer at caption size; correct in 19 languages beats compact
in one.
Signed-off-by: Rene <diazdesandi@proton.me>
* fix(menubar): harden source-PID identity resolution
Folded from 2 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(menubar): don't revert a source PID to a process that has exited
The reconciliation guard prefers a cached sourcePID over a freshly
resolved one, because SourcePIDCache's spatial matching can mis-match
when AX positions lag CG updates. Sound while the cached process is
alive. It had no liveness check, so when the owner had genuinely gone —
SourcePID changed for windowID 282: 1037 -> 1090, reverting to previous PID
— Thaw pinned the item to a dead PID and every event addressed to it
went nowhere. Both triggers appear in the #854 logs: an item's owner
relaunching, and every status item being recreated at once (window IDs
shifting by a constant 227 across two generations 24 seconds apart).
previousPIDIsLive uses the same kill(pid, 0) probe postMoveEvents
already makes before addressing a target, so the two agree on what alive
means. EPERM counts as alive: the process exists, it is just not ours to
signal. Only ESRCH is gone, and then the fresh resolution is accepted —
there is nothing left to protect.
Refs: #854
Signed-off-by: Rene <diazdesandi@proton.me>
fix(menubar): resolve an item whose title is exactly its owner's bundle ID
Ten items in a single #854 log carried their owner's bundle identifier as
their title and still had a nil source PID:
com.apple.controlcenter:com.microsoft.OneDrive
com.apple.controlcenter:com.apple.TextInputMenuAgent
com.apple.controlcenter:us.zoom.xos
com.apple.controlcenter:com.adobe.acc.AdobeCreativeCloud
…and six more
Every pass declined them. The spatial CG-to-AX pass needs an
AXExtrasMenuBar the app does not publish. Marker-pairing excludes them by
construction — it handles icons whose title is *not* bundle-ID-shaped.
The hosted-extras pass finds the right app by title and then requires
spatial confirmation against that app's AX children, which an item hosted
by Control Center cannot supply, that being the reason it is unresolved.
So a window stating who owns it went unattributed.
titleIndicatesOwner is a relation: it accepts prefixes and near-misses,
so its callers must corroborate it, and corroboration is what these items
cannot provide. exactlyNamedOwner is total equality instead — a window
titled with the complete bundle identifier of a running application is
naming its owner, not resembling it — so it needs no corroboration. It
requires a unique match, and Thaw and Control Center are excluded, so the
misattribution the other passes guard against stays guarded.
Runs before the hosted-extras pass, as the strongest signal available.
Nothing that resolves today changes: the pass only ever consults windows
already in unresolvedWindows.
This is the naming half of these reports. The moving half is the
window-owner event targeting behind postMoveEventsToWindowOwner; an item
resolved here also becomes persistable, which that flag alone cannot do.
Refs: #854, #905, #923
Signed-off-by: Rene <diazdesandi@proton.me>
* fix(menubar): keep anchored move planning off Thaw's own dividers
Folded from 4 commits; full history preserved on
archive/fix-menubar-reliability-v1. Original messages:
fix(menubar): keep rightOfAnchor order when the anchor is outside the section
#919 preserved the order of unmanaged items sharing one rightOf anchor by
counting insertions and advancing each item past the last. That assumes
the computed slot survives the clamp into the named section.
It does not when the anchor lives outside that section. Every slot then
clamps to the same section start, the counter changes nothing, and each
item is inserted ahead of the one before it — reproducing the reversal
#919 set out to fix. With an anchor in visible and two rightOf placements
naming alwaysHidden, the second item lands at index 4 and the first at 5.
Holding the previous landing site per anchor and requiring the next to
follow it makes the guarantee independent of whether the clamp fired. The
insertion counter is left in place: in the unclamped path the two agree,
and keeping it means #919's reasoning still reads straight.
Signed-off-by: Rene <diazdesandi@proton.me>
fix(menubar): abandon a move that is walking its own anchor across the bar
destinationIsStale measures one attempt against a display-width
threshold. That catches a target which jumped sections — #881's -4222 to
794 — and nothing smaller. A move fails a different way too.
The item inserts on the wrong side of its anchor: dropped at the anchor's
leading edge it can land just past it, one point to the right. The
ordinal landing check correctly refuses that, and because the menu bar
lays out right to left, the insertion shoves the anchor further left. The
next attempt re-plans against the anchor's new position and shoves it
again. Steps of 5, 13, 11 and 3 points never approach the stale
threshold, the item never lands, and the whole attempt budget goes into
walking the anchor.
Measured live: anchor minX 1682 → 1677 → 1664 → 1653 → 1650 across five
attempts while the moved item sat at 1683 throughout. When the anchor is
one of Thaw's own dividers, repeating that across cycles walks it
offscreen until the hidden section reads as zero width — at which point
saveSectionOrder and applySavedLayout both refuse, nothing persists, and
every item is visible after a restart (#924, #927).
targetIsRetreating watches the direction rather than the distance. One
nudge is expected: landing beside a target moves it by roughly the moved
item's width. Three in the same direction with no landing between them is
not reflow, it is the move pushing its anchor, and the right answer is to
stop and let the next cache tick re-plan against a settled bar — the same
answer, and the same error, the stale gate already gives.
Deliberately not changing where the drop point is computed. The same rule
lands other moves on the first attempt, so the ambiguity is conditional
on the anchor, and guessing at it would risk the moves that work today.
This stops the damage; the wrong-side insertion still wants its own fix.
Refs: #900, #924, #927
Signed-off-by: Rene <diazdesandi@proton.me>
fix(menubar): stop planning moves anchored on Thaw's own dividers
The hidden and always-hidden dividers are filtered out of the sequences
the LCS planner works on, but the chevron is not: its position within the
visible section is part of the layout and is persisted, so it has to stay
in the order. That also left it selectable as a move anchor.
Anchoring on one of Thaw's dividers is the worst case for a move that
fails. The item inserts on the wrong side, the ordinal landing check
refuses it, and because the menu bar lays out right to left the insertion
shoves the anchor further left — then the next attempt re-plans against
the new position and shoves it again. Measured live: an anchor driven
1682 → 1650 across five attempts. Walking a divider that way ends in a
zero-width hidden section, at which point saves and applies are both
refused and the layout stops persisting (#924, #927).
The divider stays in the sequence and keeps its position; it is only
barred from being chosen as an anchor. A neighbouring app item is an
equally good insertion point…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses all seven unresolved review findings on #889 without expanding the parent PR behavior:
Parent PR
This is a stacked PR targeting
fix/menubar-reliability. Merge this first, or cherry-pick commit730e1715, so #889 receives the fixes without unrelated history.Closes: N/A
Validation
730e1715swift build --package-path ThawCtlgit diff --checkRisk and rollback
No persisted-data migration or default-setting change. The gesture option remains off by default. Roll back by reverting
730e1715.