Skip to content

Notification surface consolidation + icon & keyboard-focus fixes [4/4] - #1506

Open
theboringhumane wants to merge 13 commits into
stack/03-audit-remediationfrom
stack/04-notification-fixes
Open

Notification surface consolidation + icon & keyboard-focus fixes [4/4]#1506
theboringhumane wants to merge 13 commits into
stack/03-audit-remediationfrom
stack/04-notification-fixes

Conversation

@theboringhumane

@theboringhumane theboringhumane commented Aug 29, 2026

Copy link
Copy Markdown
Member

Stack 4 of 4. Based on #1505. Split out of #1496 for reviewability.

Summary

  • Consolidate notification surfaces; cap watcher memory; harden reply delivery — squashed from the surviving cores of the notification-peek saga: unconditional banner hold on show, OTP chin width, AXI watcher memory cap + bounded idle cadence, per-stage reply timeouts with banner-first ordering + send-failure error UI, reply auto-focus removal, structured logging, duration event param
  • Fix sender app icons — the helper's one-shot running-apps bundleID match could miss forever → grey bell instead of the app icon. Now: direct /Applications probe + bounded name scan (memoized) in the helper, plus app-side re-resolution in SystemNotificationManager.add(). Adds BundleIDResolverTests (8) + ExpandedViewPixelTests
  • Stop banner keep-alive from stealing keyboard focus while closed — the 2.5s keep-alive expand seized keyboard focus even parked off-screen; it's now gated on the notch being open (refcounted setNotchOpen XPC feed, collapse on close). Hardened after review: refcount balanced on window teardown (screen lock / display-set change), gate state always re-announced on XPC reconnect
  • Park banners only while the notch is open, restore window origins (review fix from Notification live activity on the notch [1/4] #1503) — the shared Notification Center window is no longer moved when the notch is closed; original positions are recorded per window and restored on release/expiry/stop
  • Reply path hardening (review fix from Notification live activity on the notch [1/4] #1503) — reply runs on a dedicated serial queue (helper main never blocks), bounded field-wait replaces the fixed 400ms sleep, post-send verification poll gates the success report; Messages sends resolve chats first and send into the chat (account/service/handle/thread preserved), refusing to guess on ambiguity
  • Hold each banner once — dropped the redundant holdSystemBanner in show() (every path in was already held at arrival)
  • Signing fixes — test target signs JPWMG84CH8 so bare xcodebuild test loads the bundle; app-target DEVELOPMENT_TEAM restored after the Xcode churn commit had re-cleared it (reapplying ca4c328's intent)
  • XPCHelperClient hygiene — dead surface dropped, lastError wired at every failure site, lunarListener lock-synchronized, stale-invalidation identity guard
  • chore: Xcode project normalization

Notes for reviewers

Test plan

  • xcodebuild -scheme boringNotch build at every commit in this PR — clean
  • 13 targeted tests pass with a BARE command (no signing overrides): NotchUIEventTests, PlaybackStateTests, ExpandedViewPixelTests, BundleIDResolverTests (8)
  • Manual: banner while typing in another app → focus never leaves your app; open notch → reply works; lock screen while open → unlock → gate still balanced

theboringhumane and others added 13 commits August 30, 2026 12:53
…elivery

- Consolidate notification surfaces into one flow: banner -> chin
  pill -> expanded card, with an unconditional hold in show() so a
  superseded banner never parks off-screen unreleased
- Widen the OTP chin width to clear the hardware notch
- Cap AXI watcher memory pressure (autoreleasepool per scan) and
  reinstate a bounded 0.5s idle cadence
- Reply delivery: per-stage timeouts with banner-first ordering,
  and a send-failure error state ("Message couldn't be sent. Your
  draft is still here - try again or open Messages.")
- Remove the reply-focused auto-focus hijack from the notification
  live activity
- Structured logging for notification filtering decisions
- Add a duration parameter to the sneak-peek UI event

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
The XPC helper's one-shot running-apps match could leave bundleID
nil forever (renamed app, helper process owning the banner, an app
that quit between posting and capture), so every notification
surface fell back to a grey bell instead of the app icon.

Resolution now falls back to a direct /Applications probe and a
bounded name scan (memoized, hits and misses), and
SystemNotificationManager re-resolves app-side in add() when the
helper's bundleID is missing. A helper-provided bundleID always
wins.

Adds BundleIDResolverTests (8 tests: normalization parity, direct
probe, directory scan, memoization) and ExpandedViewPixelTests, a
render regression guard for the expanded notification card.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
The helper keeps held banners alive by re-performing their details
toggle every 2.5s; the toggle expands the banner, and the expanded
reply field seizes keyboard focus even parked off-screen. It fired
on every arrival regardless of notch state.

- Feed the helper the effective notch-open state (refcounted across
  screens, re-announced after XPC reconnect) via a new
  fire-and-forget setNotchOpen message
- Skip the keep-alive expand while the notch is closed; banners
  then dismiss naturally (reply requires opening the notch)
- Best-effort collapse of expanded held banners on open->closed so
  focus releases immediately
- Parking held banners off-screen continues unconditionally

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
…s/xcstrings reordering)

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
The boringNotchTests target inherited the macOS default ad-hoc identity,
so whenever the app target is signed with the real team cert the test
bundle's Team ID differs from its TEST_HOST and dlopen refuses to load it
('different Team IDs') before any test runs — every test invocation needed
command-line overrides CODE_SIGN_IDENTITY/DEVELOPMENT_TEAM to pass.

Give the test target the app target's ca4c328 choice explicitly:
CODE_SIGN_IDENTITY = Apple Development, DEVELOPMENT_TEAM = JPWMG84CH8
(automatic style). Bare 'xcodebuild test' now loads the bundle and runs
green with no overrides; CI can still override at build time.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
add() holds every notification at arrival — before the enqueue branch —
and every path into show() (add, promoteNextQueued, cycleToNextQueued)
passes one of those already-held notifications. The second hold in show()
was a no-op against the helper's held set and an extra XPC round-trip per
banner. The add() hold stays: it covers the enqueue path too.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
8cbb4fc accepted Xcode's project normalization, which silently re-cleared
the app target's DEVELOPMENT_TEAM and re-added the sdk-qualified ad-hoc
identity that ca4c328 had removed. Restore the team on the app target
(Debug + Release); the helper target's long-standing ad-hoc config is
unchanged. App and test targets now both sign JPWMG84CH8, so bare
xcodebuild build/test works with real identities.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
Review of the XPCHelperClient merge found the refcount could leak:
cleanupWindows() (screen lock with showOnLockScreen off, or the
all-displays toggle) tears windows down without closing the view model,
so an open notch never fires its open->closed onChange and the helper's
focus gate would stay stuck open forever — silently re-enabling the
keyboard-focus steal the gate exists to prevent.

- ContentView gains a state-guarded .onDisappear that decrements when a
  view dies while its notch is open (balanced against the onChange path;
  no double-decrement since a prior close clears the state first)
- ensureRemoteService now always re-announces the effective gate state
  on a fresh connection instead of only re-sending true, so helper state
  converges to ours after helper restarts or app-quit-while-open

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
…ze lunarListener

1. Remove dead private getRemoteService() — zero callers repo-wide.
2. Remove vestigial write-only hasLunarListener (property + all writes);
   the needsListener parameter existed only to feed it, so it goes too.
3. Make lastError truthful: every catch and former try? site now records
   .transport(underlying:) — MainActor sites set it directly, nonisolated
   sites hop via MainActor.run. Header comment rewritten to describe the
   actual behavior. No control flow or return values changed.
4. Remove zero-call-site APIs at all three layers: dismissNotification,
   isScreenBrightnessAvailable, isKeyboardBrightnessAvailable from client,
   protocol, and helper @objc wrappers (plus the now-unused
   KeyboardBrightnessClient.isAvailable). NotificationWatcher.dismiss
   stays — shared watcher internals are out of this pass's scope.
5. Synchronize NotificationXPCDelegate.lunarListener with an NSLock-
   guarded backing property: written on MainActor, read on the XPC
   delivery queue; listener still invoked on the delivery queue.
6. Fix helperAvailable transient: new connectionInterrupted flag set by
   interruption/invalidation hops, cleared when a fresh connection is
   stored; existing-connection path reports helperAvailable =
   !connectionInterrupted instead of unconditional true.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
Audit of the hygiene commit found the connectionInterrupted flag was
observably inert: the interruption hop nils remoteService in the same
MainActor task that sets the flag, so the existing-connection path can
never read it true. Remove the dead logic instead of layering more
state on a one-runloop transient.

The same review surfaced a real pre-existing bug: the interruption and
invalidation handlers nilled connection/remoteService with no identity
check, so a stale handler from a deallocated connection could wipe a
freshly-built one. Both handlers now capture their connection weakly
and bail unless it is still the current one.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
Reply into the originating conversation instead of the first global
participant match: scan chats by their participants' display names,
prefer a 1:1 chat (preserves account/service/handle/thread), then a
matching group chat, and only fall back to a bare participant when
exactly one matches library-wide. Zero or 2+ participant matches now
return distinct notfound/ambiguous statuses and map to false, so a
wrong-person/wrong-thread/wrong-transport send can never report ok.

Addresses Alexander5015's review on PR #1503.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
…f main queue

hold() inserted into held unconditionally but moved Notification Center's
shared window off-screen unconditionally too, hiding unrelated banners
and never restoring the position (RC1). The park is now gated on
notchOpen like the keep-alive toggle, the window's origin is recorded on
first park keyed by CFHash(window), and the position is restored when
the last parked hold on that window ends (release, banner-gone, stop).

reply() drove the banner's async AX hierarchy with a fixed 400ms
Thread.sleep on the helper's main queue — stalling the banner poll and
hold refresh — and equated AX action success with delivery (RC4). The
reply path now runs on a serial replyQueue (the XPC wrapper completes
from there), waits bounded 50ms slices for the reply field to appear
(~1.2s cap), and after the send verifies the banner accepted the reply
by polling for the field to clear or disappear (~1s cap), reporting
false otherwise. AX still cannot verify network delivery; documented.

Co-authored-by: TheBoringMajdoor <themajdoor@theboring.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant