fix(desktop): always show brand marks for onboarding integration icons#10210
Merged
Conversation
The onboarding "Use Omi memory where you work" rows (and the matching home stack) resolved Notion/ChatGPT/Claude icons only from a hardcoded /Applications path, so any user without the app installed at exactly that path saw a generic gray SF Symbol instead of a brand mark. - Ship bundled notion_logo/chatgpt_logo/claude_logo PNGs (Simple Icons brand marks, recolored for the dark tile); Codex reuses the ChatGPT mark and Claude Code the Claude mark, matching the app-icon reuse. - Resolve installed apps by bundle identifier via LaunchServices first (finds ~/Applications, Setapp, renamed copies), keeping the fixed path as fallback. - Harden the SwiftPM resource-bundle probe to scan nested "<pkg>_<target>.bundle" locations under every candidate bundle, so the bundled-logo fallback also resolves outside the packaged app layout (and in tests). Verification: xcrun swift build -c debug clean; ConnectorBrandIconResourceTests (new) passes — every onboarding brand resolves a loadable bundled logo with no brand apps installed on the build machine.
skanderkaroui
force-pushed
the
fix/onboarding-brand-icons
branch
from
July 21, 2026 17:50
359773d to
4a06b07
Compare
…oval 4a82447 removed typing from the floating bar (pill steering now routes to the main app via Continue in Omi) but updated none of the source-inspection tripwires anchored on the deleted composer, so Desktop Swift CI is red on main and the fail-closed release candidate gate is blocked. Re-encode the new contract: - testProviderRequestsStayInTheModelLoop: the in-model-loop anchor is now the Continue-in-Omi route to openMainAppWindow(); the no-local- parsing negatives are unchanged. - testSubagentComposerOnlyContinuesItsCanonicalSession → testSubagentFollowUpsOnlyContinueTheCanonicalSession: anchors on the manager's canonical-session guard and DesktopCoordinatorService.continueAgent, since the view composer no longer exists. - testTypedSendDelegatesResponseSizingToWindow: archiving moved into the window with sizing; assert the window archives via historyChatProvider and the view no longer archives at all. - FloatingBarTimingSignalTests.testFollowUpFocusUsesViewLifecycle: deleted — the follow-up field it guarded no longer exists; the asyncAfter baseline test still guards the timing-signal class. Verification: xcrun swift test --filter "AgentPillLifecycleTests|FloatingBarTimingSignalTests" — 77 tests, 0 failures (was 4 failures, matching the red Desktop Swift CI run on main at 928b396); scripts/check_desktop_test_quality.py at/below baseline; agent-logic-harness filter references only class names, and the class still exists.
…artifact refactor dba3af2 (restore single macOS artifact) removed the beta production identity from AppBuild while #10216 was in flight; #10216 then landed INV-DATA-1 plus guards that pin com.omi.computer-macos.beta to the production customer data plane. Their merge left main red and, worse, routed installed Omi Beta apps to the development backend. Restore the boundary the locked invariant demands: - AppBuild: betaProductionBundleIdentifier + productionFamilyBundleIdentifiers are back; isNonProduction / isProductionBundle key off the family set. - DesktopBackendEnvironment: production-family bundles (not just stable) fall through to production routing, so installed beta apps keep their customer data plane. - Re-restore the stable-only legacy Omi Computer.app cleanup gate from d5a8ba9 (also dropped by dba3af2): with beta production-family again, an ungated cleanup from a beta install would force-terminate the running stable app. - AppBuildProductionFamilyTests covers the family set and the cleanup gate. The retirement of the separately packaged beta artifact (channel pointer over immutable Omi.zip/omi.dmg) is untouched: this restores identity and routing semantics for already-installed beta bundles only.
skanderkaroui
added a commit
that referenced
this pull request
Jul 21, 2026
…ed composer testFollowUpFocusUsesViewLifecycle pinned the floating-bar follow-up composer that #10181 deleted, so it fails on every merge with main. Same deletion as the repair carried by #10210/#10204. Verified: xcrun swift test --filter "FloatingBarTimingSignalTests|AgentPillLifecycleTests" — 77/77 pass.
… single-artifact refactor" This reverts commit 08a4016.
Git-on-my-level
added
the
needs-maintainer-review
Collaborator
|
Thanks @skanderkaroui — the brand-icon fix itself looks good: bundled logos for Notion/ChatGPT/Claude (Codex reuses ChatGPT, Claude Code reuses Claude), LaunchServices bundle-id lookup with the Two things for a human maintainer before merge:
CI is not green at this head: No formal approval from me under the gate — the core fix is sound, but the carried unrelated test repair plus the reverted beta churn make this a maintainer-sign-off call rather than an auto-approve. Labeling by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
undivisible
added a commit
that referenced
this pull request
Jul 21, 2026
…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. -->
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
kodjima33
approved these changes
Jul 22, 2026
kodjima33
left a comment
kodjima33
left a comment
Collaborator
There was a problem hiding this comment.
fix(desktop): always show brand marks for onboarding integrations — desktop bug fix; held (desktop-swift red)
…icons # Conflicts: # desktop/macos/Desktop/Tests/AgentPillLifecycleTests.swift
Git-on-my-level
added a commit
that referenced
this pull request
Jul 22, 2026
…n redesign (#10340) ## Summary Restores onboarding functionality that the Second Brain redesign (#10218) dropped, re-implemented on top of the new conversational design (no old UI brought back). Each fix is its own commit: - **Auto-detect user name** (#9919) — seed the name field from the known `givenName`, kick a backend name fetch, and adopt an async name arrival (Apple sends the name only on first auth), so onboarding greets by name instead of a generic placeholder. Also re-hydrates name/language/role drafts on resume. - **Clear onboarding state on sign-out** (#10108) — `sbOnboardingResumeStep` + `onboardingRole` were added by the redesign but left out of the shared clearing list, leaking the prior user's resume step + role to the next account on the same Mac. Added both, plus a regression test. - **Drag-to-grant Full Disk Access** (#9742) — the drag-to-grant helper was wired for Screen Recording but not FDA; now both use it. - **Connector brand marks + Claude Code restart** (#10210 / #10205) — connect rows show each connector's brand icon, and Claude Code shows a restart prompt once connected so running sessions reload the new MCP config. - **Remove post-onboarding walkthrough** — the coach-mark tour that ran after onboarding (spotlighting each nav pill) is deleted; a just-onboarded user lands straight on Home. - **Pin the drag-to-grant card under System Settings** — the permission drag card was pinned to the screen's bottom quarter; it now anchors directly beneath the live Settings window (flipping above it when there's no room below) so it follows the window and never covers the drop target. ## Verification Build/manual verification pending — depends on #10334 (restores the `Package.swift`/`Package.resolved` that a stray "fixture" commit gutted on `main`, which currently breaks the desktop Swift build). This PR carries no Package changes; once #10334 lands and this rebases, it builds on the repaired manifest. ## Product invariants affected - INV-INT-1 ## Failure class (fixes) Failure-Class: none <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/10340?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. -->
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
3 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.
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>.apppath, so any user without the app installed at exactly that path saw a generic gray SF Symbol tile.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.installedApplicationURLnow asks LaunchServices (urlForApplication(withBundleIdentifier:)) first — finds apps in~/Applications, Setapp, or renamed copies — keeping the fixed/Applicationspath as a fallback for unregistered copies."<pkg>_<target>.bundle"scan now checks nested bundle locations under every candidate bundle (not justBundle.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.ConnectorBrandIconResourceTestspasses (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.make preflightgreen.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-routingguard rejects the divergent identity, so that commit is reverted here — the branch no longer touches AppBuild/DesktopBackendEnvironment/OmiApp.Product invariants
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.)