fix(desktop): repair Home redesign UI regressions (hub Esc/click trap, keyboard-invisible chat actions, Continue-in-Omi landing)#10211
Conversation
…creen (#10204) Fixes #10143 — an active screen share in a call (Zoom/Teams/Meet) stops at the moment Omi takes its periodic Rewind screenshot. ## Root cause The Rewind capture path issues one-shot `SCScreenshotManager.captureImage` requests every ~1s. When another app is streaming the screen (an outgoing call share), the concurrent ScreenCaptureKit request contends in WindowServer capture arbitration; this is the same cause family as the CleanShot/Shottr 20–60s stall fixed by #6819, but the observed outcome — the user's share terminating — is worse. Apple documents no contract for concurrent capture (checked SCScreenshotManager/SCStream docs), so the mitigation is to not capture at all while a share is active. ## Fix - `ConferencingApps.isShareIndicatorWindow(ownerName:title:)` + `activeScreenSharePresent()`: detect the share-only chrome windows conferencing apps create while the user is presenting (Zoom "zoom share statusbar/toolbar window" floating controls, Teams "Screen sharing toolbar", the browser "<site> is sharing your screen/a tab/a window" stop-sharing bubble — this covers Google Meet and Teams web). Adding another app is a 2-line signature + test. - New `ProactiveExternalCaptureYield` (Core): one seam that runs both "yield to an external capture" gates — the existing screenshot-app gate (#6819) and the new share gate — each a `ProactiveScreenshotCaptureGate` with its own 10s post-condition backoff. The share check is an autoclosure so its CGWindowList enumeration is skipped when the screenshot gate already pauses the tick. Extracted (rather than grown in place) because `ProactiveAssistantsPlugin.swift` is frozen by the product file line-count ratchet; the plugin now makes a single `shouldYield` call. - Declares a new failure class, `FC-concurrent-capture-contention` (definition added in this PR, as `Failure-Class: new` requires): second instance of this cause after #6819; the reusable guard surface is the shared gate state machine + the new yield seam. Failure-Class: new ## Why per-app window signatures instead of a native "screen is being shared" API macOS has **no public API** that reports another app capturing/sharing the screen. `UIScreen.isCaptured` is iOS/Catalyst-only; the only macOS equivalent is the private SkyLight call `CGSIsScreenWatcherPresent()`. Even if we accepted a private API, any generic "screen watcher present" signal is self-confounded: **Omi itself is a screen watcher every ~1s**, so the signal would read true because of our own capture, not the call app's share. **Follow-up worth validating live** (needs a GUI session): whether one-shot `SCScreenshotManager` captures do *not* light the macOS 15 system capture indicator while persistent `SCStream` shares do. If that holds, enumerating the system indicator window becomes an app-agnostic signal covering Discord/Slack/any social app — qualified by a persistence threshold (N consecutive ticks) so a short-lived capture (a screenshot tool, a one-shot grab) is never misread as a video share. The `ProactiveExternalCaptureYield` seam is where that signal would plug in. ## Carried CI repair `4a8244742d` (#10181) removed floating-bar typing but left source-inspection tripwires anchored on the deleted composer (AgentPillLifecycleTests, FloatingBarTimingSignalTests), so the desktop Swift test target fails on every merge with main. This branch carries the tripwire re-anchor commit; the same repair rides in #10210, and #10211 carries an equivalent one — whichever merges first repairs main and the later copies merge as no-ops. The branch previously also carried a beta production-identity restore for INV-DATA-1; main has since been repaired canonically (beta is a channel on the production bundle id, #10219) and the updated `production-data-plane-routing` guard rejects the divergent identity, so that commit is reverted here — the branch no longer touches AppBuild/DesktopBackendEnvironment/OmiApp. (The single Failure-Class declaration above stays `new` for this PR's own FC-concurrent-capture-contention definition.) ## Verification - `xcrun swift build -c debug --package-path Desktop` — clean build. - `xcrun swift test --filter "ConferencingAppsTests|ProactiveAssistantOrchestrationPolicyTests"` — 23/23 pass: new share-indicator classification tests (positive Zoom/Teams/browser signatures; negatives for ordinary call windows, a Teams chat titled about screen sharing, non-conferencing owners) and new `ProactiveExternalCaptureYield` behavioral tests (pause across a share then backoff then resume; screenshot gate short-circuits the share check; reset clears both gates). - Full desktop Swift suite (`scripts/swift-test-suites.sh`, 344 suites): same result as pristine `origin/main` on this machine — 3 suites fail identically on both (AgentPillLifecycleTests, FloatingBarTimingSignalTests, ScreenRecordingPermissionPolicyTests; headless-environment failures, no GUI session), i.e. no regression from this change. - `make preflight` — 19/19 checks pass. - **Live verification (named bundle + real Google Meet share)**: built this branch as a named test bundle and ran a real Meet screen share against it. Log shows `Active screen share detected — pausing capture until the share ends` at share start, zero capture activity for the full ~2.5-minute share, `Screen share ended` on stop, and the encoder/indexer resuming frames ~9s later. The Meet share survived the entire session — the #10143 failure mode did not reproduce. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/10204?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
…hover-only actions, Continue-in-Omi landing, knows-list dedup) Four bugs introduced by the Home redesign (#10184) and floating-bar typing removal (#10181): - Hub click/Esc trap: the collapse catchers mounted whenever homeMode != homeRestingMode, which with chat history present put them OVER the hub — a stray click or Esc then *opened* the chat. Catchers now mount only over a non-resting panel, never the hub (HomeStageMode.collapseCatcherActive). Also corrected the autoOpenChatForExistingHistoryIfNeeded comment, which described an 'Esc back to the hub' that cannot exist once history is present. - Chat message actions/timestamps were opacity-gated on pointer hover only, leaving Tab / Full Keyboard Access focused on invisible buttons. The metadata row now also reveals while any of its controls holds keyboard focus (shared FocusState + ChatBubbleMetadataReveal). - 'Continue in Omi' only called openMainAppWindow(), landing the user on whatever tab the main window last showed. Both floating-bar affordances now route through openMainAppChat(): a one-shot MainChatNavigationRequestStore request switches DesktopHomeView to the Home tab and DashboardPage opens the chat panel on mount or in place. - HomeKnowsListComposer derives question-row IDs from text; duplicate suggestions produced colliding ForEach IDs. Questions are deduped. Failure-Class: none Verification: - xcrun swift build -c debug --package-path Desktop — clean. - xcrun swift test --filter 'HomeKnowsComposerTests|HomeStageCollapseCatcherTests|MainChatNavigationRequestStoreTests|ChatBubbleMetadataRevealTests' — 15/15 pass (new regression tests for all four fixes). - Guard suites ChatTimelineContinuityTests, DashboardCaptureStateTests, HomeSuggestionsStoreTests, HomeStatusStoreTests — pass. - desktop-core-harness.sh --self-check and check-e2e-flow-coverage.py --strict — pass (MainChatNavigationRequest.swift covered by home-stage.yaml). - Live named-bundle exercise was attempted but is blocked on this machine: no signed-in session to seed, no redis/docker for the hermetic T2 stack, and the Auth-emulator local profile launch wedges in SecItemAdd under the console-user launch path. The behavior changes are covered by the unit regression tests above; the real user-facing paths were NOT exercised end-to-end.
…mposer Three AgentPillLifecycleTests source tripwires still pinned the typed follow-up composer that #10181 deleted, and have been red on main since it merged: testProviderRequestsStayInTheModelLoop and testSubagentComposerOnlyContinuesItsCanonicalSession expected manager.continueAgent(from: pill, ...), and testTypedSendDelegatesResponseSizingToWindow expected the view to call state.archiveCurrentExchange (it moved into FloatingControlBarWindow's query paths). The tripwires now pin the replacement contract: the composer's only affordance is openMainAppChat(), no view-side archive, no second send path. Verification: xcrun swift test --filter AgentPillLifecycleTests — 75/75 pass (was 72/75 on clean main; reproduced the 3 failures on main at 476feab before this change).
ChatBubble 1926->1952 (keyboard-focus reveal), DashboardPage 4543->4582 (catcher predicate + chat-landing consume + lifecycle chain split), OmiApp 1645->1653 (openMainAppChat). Justifications updated in the baseline files.
1d97822 to
f852604
Compare
|
Thanks for tightening these Home redesign regressions — the fixes are cohesive and the static diff lines up with the reported behavior:
I’m not formally approving yet because the current head has failing required desktop checks ( No security/supply-chain concern found in the reviewed diff. Once CI is green, this looks like a good focused regression repair for maintainer review. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
kodjima33
left a comment
There was a problem hiding this comment.
fix(desktop): repair Home redesign UI regressions — desktop bug fix; held (desktop-swift red)
# Conflicts: # desktop/macos/Desktop/Sources/MainWindow/Pages/DashboardPage.swift
#10210) ## What Onboarding integration icons (the "Use Omi memory where you work" step, and the matching home stack — same `ConnectorBrandIcon`) didn't show up for most users: Notion, ChatGPT, and Claude marks resolved **only** from a hardcoded `/Applications/<App>.app` path, so any user without the app installed at exactly that path saw a generic gray SF Symbol tile. - **Bundled brand marks for every onboarding row.** Ships `notion_logo.png`, `chatgpt_logo.png`, `claude_logo.png` (Simple Icons brand marks, rasterized at 512px with alpha and recolored for the dark tile: white Notion/OpenAI, Claude coral `#D97757`). Codex reuses the ChatGPT mark and Claude Code the Claude mark, mirroring the existing app-icon reuse. Every destination the onboarding introduces (Notion, Obsidian, Claude / Claude Code, ChatGPT / Codex, OpenClaw, Hermes, plus Gemini behind "More") now has a real logo with zero apps installed. - **Installed apps found by bundle identifier.** `installedApplicationURL` now asks LaunchServices (`urlForApplication(withBundleIdentifier:)`) first — finds apps in `~/Applications`, Setapp, or renamed copies — keeping the fixed `/Applications` path as a fallback for unregistered copies. - **Sturdier resource-bundle probe.** The SwiftPM `"<pkg>_<target>.bundle"` scan now checks nested bundle locations under every candidate bundle (not just `Bundle.main.resourceURL`), so the bundled-logo fallback also resolves outside the packaged-app layout — this is what lets the new regression test load the real resources hermetically. ## Verification - `xcrun swift build -c debug --package-path Desktop` — clean. - New `ConnectorBrandIconResourceTests` passes (`xcrun swift test --filter ConnectorBrandIconResourceTests`): every onboarding brand resolves a loadable bundled logo. The build machine has none of Notion/Obsidian/ChatGPT/Claude installed, so this exercises exactly the previously-broken "no apps installed" scenario end-to-end through the production loader. - Rendered-logo visual check: composited all three new PNGs on the dark tile background and inspected — crisp white OpenAI knot, coral Claude spark, white Notion mark. - `make preflight` green. ## Carried CI repair `4a8244742d` (#10181) removed floating-bar typing but left source-inspection tripwires anchored on the deleted composer (AgentPillLifecycleTests, FloatingBarTimingSignalTests), so the desktop Swift test target fails on every merge with main. This branch carries the tripwire re-anchor commit; the same repair rides in #10204, and #10211 carries an equivalent one — whichever merges first repairs main and the later copies merge as no-ops. The branch previously also carried a beta production-identity restore for INV-DATA-1; main has since been repaired canonically (beta is a channel on the production bundle id, #10219) and the updated `production-data-plane-routing` guard rejects the divergent identity, so that commit is reverted here — the branch no longer touches AppBuild/DesktopBackendEnvironment/OmiApp. ## Product invariants - INV-INT-1 — touched `ConnectorBrandIcon.swift` (matches `**/*Connector*`). Icon rendering only: no connector contract, probe, or "Connected" semantics change. The bundle identifiers added are stable app identity (same class of fact as the pre-existing hardcoded install paths), not volatile provider knowledge. ## Failure class (fixes) Failure-Class: none (The icon fix introduces no failure class; the previously declared FC-customer-data-plane-divergence rode with the carried data-plane repair, which is now reverted in favor of main's canonical fix from #10219.) <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/10210?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
Summary
Fixes four UI regressions introduced by the Home redesign (#10184) and the floating-bar typing removal (#10181), plus repairs three
AgentPillLifecycleTeststripwires that have been red onmainsince #10181 merged.homeMode != homeRestingMode, which with chat history present put them over the hub — a stray click or Esc then opened the chat (Esc entering a screen). Catchers now mount only over a non-resting panel, never the hub (HomeStageMode.collapseCatcherActive, unit-tested). Also corrected theautoOpenChatForExistingHistoryIfNeededcomment that described an "Esc back to the hub" which cannot exist once history is present (its real purpose is not overriding the bridge'shome_close_panel).FocusState+ChatBubbleMetadataReveal, unit-tested).openMainAppWindow()only, dropping the user on whatever tab the main window last showed. They now route throughopenMainAppChat(): a one-shotMainChatNavigationRequestStorerequest switchesDesktopHomeViewto the Home tab andDashboardPageopens the chat panel on mount or in place. The deliberately-relabeled "Open Omi" shortcut/button are unchanged.HomeKnowsListComposerderives question-row IDs from text; duplicate suggestions produced collidingForEachIDs. Questions are now deduped (unit-tested).testProviderRequestsStayInTheModelLoop,testSubagentComposerOnlyContinuesItsCanonicalSession, andtestTypedSendDelegatesResponseSizingToWindowstill pinned the typed composer Desktop chat: model-decided thinking effort; floating bar typing removed (Open Omi shortcut) #10181 deleted — reproduced all three failing on cleanmainat 476feab. They now pin the replacement contract (composer routes viaopenMainAppChat(); archive owned by the window's query paths).Product invariants
ChatBubblemetadata-row reveal adds keyboard focus as a reveal trigger. No write-path, provider, projection, or identity changes;ChatTimelineContinuityTestsandAgentPillLifecycleTestspass.OmiApp.swiftis touched only to addopenMainAppChat()(window reveal + navigation request); no session/auth lifecycle changes.desktop-auth-session-ratchetpasses.Failure-Class: none
Verification
xcrun swift build -c debug --package-path Desktop— clean.xcrun swift test --filter 'HomeKnowsComposerTests|HomeStageCollapseCatcherTests|MainChatNavigationRequestStoreTests|ChatBubbleMetadataRevealTests'— 15/15 pass (new regression tests covering all four fixes).ChatTimelineContinuityTests,DashboardCaptureStateTests,HomeSuggestionsStoreTests,HomeStatusStoreTests— pass.AgentPillLifecycleTests— 75/75 pass (72/75 on cleanmain; the 3 failures reproduce there without this branch)../scripts/desktop-core-harness.sh --self-check— pass (56 flows, 132 actions).python3 scripts/check-e2e-flow-coverage.py --strict— Uncovered: 0 (MainChatNavigationRequest.swiftcovered byhome-stage.yaml).make preflight— green with this body.SecItemAddunder the console-user launch path). A manual pass on a signed-in machine before merge is recommended: (1) with chat history, bridgehome_close_panelthen stray-click/Esc must stay on the hub; (2) floating-bar "Continue in Omi" from a non-Home tab must land on the Home chat; (3) Tab through a chat message must reveal its action row.