Add local desktop automation bridge and managed Sparkle updates#5635
Closed
Add local desktop automation bridge and managed Sparkle updates#5635
Conversation
Collaborator
Author
|
Superseded by #5636, which is branched directly from main and contains only this change set. |
Contributor
|
Hey @kodjima33 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! 💜 |
Contributor
Greptile SummaryThis PR adds two features to the macOS desktop app: (1) a local HTTP automation bridge for programmatic app state inspection and in-app navigation, and (2) managed Sparkle auto-update behavior that enforces automatic checks and installs on release builds while keeping dev builds opt-out.
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Agent as External Agent
participant Bridge as DesktopAutomationBridge<br/>(TCP :47777)
participant Store as DesktopAutomationStateStore<br/>(Actor)
participant Home as DesktopHomeView
participant App as macOS App
Note over Home,Store: State Reporting Flow
Home->>Home: .onChange / NSNotification
Home->>Store: update(snapshot)
Note over Agent,App: GET /state Flow
Agent->>Bridge: GET /state
Bridge->>Store: current()
Store-->>Bridge: DesktopAutomationSnapshot
Bridge-->>Agent: JSON response
Note over Agent,App: POST /navigate Flow
Agent->>Bridge: POST /navigate {target, settingsSection}
Bridge->>App: NotificationCenter.post(.desktopAutomationNavigateRequested)
App->>Home: handleAutomationNavigation()
Home->>Home: Update selectedIndex / settingsSection
Home->>Store: reportAutomationState()
Bridge->>Store: current()
Store-->>Bridge: Updated snapshot
Bridge-->>Agent: JSON response
Last reviewed commit: f18813a |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+105
to
+112
| do { | ||
| let parameters = NWParameters.tcp | ||
| parameters.allowLocalEndpointReuse = true | ||
| guard let port = NWEndpoint.Port(rawValue: DesktopAutomationLaunchOptions.port) else { | ||
| log("DesktopAutomationBridge: invalid port \(DesktopAutomationLaunchOptions.port)") | ||
| return | ||
| } | ||
| let listener = try NWListener(using: parameters, on: port) |
Contributor
There was a problem hiding this comment.
TCP listener binds to all interfaces, not just loopback
NWListener with NWParameters.tcp listens on all network interfaces by default, not just 127.0.0.1. The log message on line 122 says "listening on http://127.0.0.1:..." but any device on the local network can connect to this port and call /state (leaking app state) or /navigate (triggering in-app navigation).
To restrict the listener to loopback only, set requiredLocalEndpoint on the parameters before creating the listener:
Suggested change
| do { | |
| let parameters = NWParameters.tcp | |
| parameters.allowLocalEndpointReuse = true | |
| guard let port = NWEndpoint.Port(rawValue: DesktopAutomationLaunchOptions.port) else { | |
| log("DesktopAutomationBridge: invalid port \(DesktopAutomationLaunchOptions.port)") | |
| return | |
| } | |
| let listener = try NWListener(using: parameters, on: port) | |
| let parameters = NWParameters.tcp | |
| parameters.allowLocalEndpointReuse = true | |
| guard let port = NWEndpoint.Port(rawValue: DesktopAutomationLaunchOptions.port) else { | |
| log("DesktopAutomationBridge: invalid port \(DesktopAutomationLaunchOptions.port)") | |
| return | |
| } | |
| parameters.requiredLocalEndpoint = NWEndpoint.hostPort(host: .ipv4(.loopback), port: port) | |
| let listener = try NWListener(using: parameters, on: port) |
Comment on lines
209
to
+212
| // Initialize Sparkle auto-updater early so the 10-minute check timer starts at launch | ||
| // Without this, the updater only starts when the user opens Settings or clicks "Check for Updates" | ||
| _ = UpdaterViewModel.shared | ||
|
|
||
| // Initialize Sentry for crash reporting and error tracking (including dev builds) | ||
| let isDev = AnalyticsManager.isDevBuild | ||
| SentrySDK.start { options in | ||
| options.dsn = "https://8f700584deda57b26041ff015539c8c1@o4507617161314304.ingest.us.sentry.io/4510790686277632" | ||
| options.debug = false | ||
| options.enableAutoSessionTracking = true | ||
| options.environment = isDev ? "development" : "production" | ||
| // Disable automatic HTTP client error capture — the SDK creates noisy events | ||
| // for every 4xx/5xx response (e.g. Cloud Run 503 cold starts on /v1/crisp/unread). | ||
| // App code already handles HTTP errors and reports meaningful ones explicitly. | ||
| options.enableCaptureFailedRequests = false | ||
| options.maxBreadcrumbs = 100 | ||
| options.beforeSend = { event in | ||
| // Allow user feedback through from all builds (dev + prod) | ||
| if event.message?.formatted.hasPrefix("User Report") == true { return event } | ||
| // Never send other events from dev builds — they pollute production Sentry data | ||
| if isDev { return nil } | ||
| // Filter out HTTP errors targeting the dev tunnel — noise when the tunnel is down | ||
| if let urlTag = event.tags?["url"], urlTag.contains("m13v.com") { | ||
| return nil | ||
| } | ||
| // Filter out NSURLErrorCancelled (-999) — these are intentional cancellations | ||
| // (e.g. proactive assistants cancelling in-flight Gemini requests on context switch) | ||
| if let exceptions = event.exceptions, exceptions.contains(where: { exc in | ||
| exc.type == "NSURLErrorDomain" && exc.value.contains("Code=-999") || | ||
| exc.type == "NSURLErrorDomain" && exc.value.contains("Code: -999") | ||
| }) { | ||
| return nil | ||
| } | ||
| // Filter out AuthError.notSignedIn — this is thrown when token refresh transiently | ||
| // fails (network blip, expired token mid-refresh). The user is still signed in per | ||
| // UserDefaults; the 30s refresh timer will retry. Not actionable as a Sentry error. | ||
| if let exceptions = event.exceptions, exceptions.contains(where: { exc in | ||
| exc.type == "Omi_Computer.AuthError" && exc.value.contains("notSignedIn") | ||
| }) { | ||
| return nil | ||
| } | ||
| return event | ||
| } | ||
| UpdaterViewModel.shared.checkForUpdatesImmediatelyAfterLaunchIfNeeded() |
Contributor
There was a problem hiding this comment.
Inconsistent indentation (8 spaces vs 4)
These four lines use 8-space indentation while the rest of applicationDidFinishLaunching uses 4 spaces. This looks like a merge artifact.
Suggested change
| // Initialize Sparkle auto-updater early so the 10-minute check timer starts at launch | |
| // Without this, the updater only starts when the user opens Settings or clicks "Check for Updates" | |
| _ = UpdaterViewModel.shared | |
| // Initialize Sentry for crash reporting and error tracking (including dev builds) | |
| let isDev = AnalyticsManager.isDevBuild | |
| SentrySDK.start { options in | |
| options.dsn = "https://8f700584deda57b26041ff015539c8c1@o4507617161314304.ingest.us.sentry.io/4510790686277632" | |
| options.debug = false | |
| options.enableAutoSessionTracking = true | |
| options.environment = isDev ? "development" : "production" | |
| // Disable automatic HTTP client error capture — the SDK creates noisy events | |
| // for every 4xx/5xx response (e.g. Cloud Run 503 cold starts on /v1/crisp/unread). | |
| // App code already handles HTTP errors and reports meaningful ones explicitly. | |
| options.enableCaptureFailedRequests = false | |
| options.maxBreadcrumbs = 100 | |
| options.beforeSend = { event in | |
| // Allow user feedback through from all builds (dev + prod) | |
| if event.message?.formatted.hasPrefix("User Report") == true { return event } | |
| // Never send other events from dev builds — they pollute production Sentry data | |
| if isDev { return nil } | |
| // Filter out HTTP errors targeting the dev tunnel — noise when the tunnel is down | |
| if let urlTag = event.tags?["url"], urlTag.contains("m13v.com") { | |
| return nil | |
| } | |
| // Filter out NSURLErrorCancelled (-999) — these are intentional cancellations | |
| // (e.g. proactive assistants cancelling in-flight Gemini requests on context switch) | |
| if let exceptions = event.exceptions, exceptions.contains(where: { exc in | |
| exc.type == "NSURLErrorDomain" && exc.value.contains("Code=-999") || | |
| exc.type == "NSURLErrorDomain" && exc.value.contains("Code: -999") | |
| }) { | |
| return nil | |
| } | |
| // Filter out AuthError.notSignedIn — this is thrown when token refresh transiently | |
| // fails (network blip, expired token mid-refresh). The user is still signed in per | |
| // UserDefaults; the 30s refresh timer will retry. Not actionable as a Sentry error. | |
| if let exceptions = event.exceptions, exceptions.contains(where: { exc in | |
| exc.type == "Omi_Computer.AuthError" && exc.value.contains("notSignedIn") | |
| }) { | |
| return nil | |
| } | |
| return event | |
| } | |
| UpdaterViewModel.shared.checkForUpdatesImmediatelyAfterLaunchIfNeeded() | |
| // Initialize Sparkle auto-updater early so the 10-minute check timer starts at launch | |
| // Without this, the updater only starts when the user opens Settings or clicks "Check for Updates" | |
| _ = UpdaterViewModel.shared | |
| UpdaterViewModel.shared.checkForUpdatesImmediatelyAfterLaunchIfNeeded() |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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
1 participant
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
Verification
xcrun swift build -c debug --package-path desktop/DesktopGET /state+POST /navigateNotes