Merge latest fleet-desktop-macos changes from upstream and bump version to v1.4.0 - #49469
Conversation
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR updates the fleet-desktop-macos app metadata to v1.4.0 and adds a single-instance strategy so duplicate launches forward fleet:// deep links (or reopen requests) to the primary instance instead of creating a second Dock icon.
Changes:
- Bump Fleet Desktop macOS version/build numbers to
1.4.0/8. - Add
LSMultipleInstancesProhibitedto the app’s Info.plist. - Implement a single-instance guard + URL/reopen handoff using
DistributedNotificationCenter.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/fleet-desktop-macos/FleetDesktop/Info.plist | Version/build bump and prohibit multiple instances at the Launch Services level. |
| apps/fleet-desktop-macos/FleetDesktop/FleetDesktopApp.swift | Add primary/secondary instance detection and distributed-notification handoff for URL opens and relaunches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @objc private func handleForwardedReopen(_ notification: Notification) { | ||
| fleetService.run() | ||
| } |
There was a problem hiding this comment.
Sounds valid enough? I'm not too sure about the threading in Swift UI
There was a problem hiding this comment.
Done — dispatched to main in a860fd4, same as the URL handler above.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe macOS app now enforces single-instance behavior, forwarding duplicate-instance reopen and 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/fleet-desktop-macos/FleetDesktop/FleetDesktopApp.swift`:
- Around line 45-47: Update the handoff flow around isAlreadyRunningElsewhere
and the related launch/reopen handling so a detected older primary without
distributed-notification support does not cause the new instance to exit while
dropping the action. Use a backward-compatible OS handoff mechanism or verify
protocol support before setting isSecondaryInstance and returning, with a
fallback that preserves the URL/reopen request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8ff1e848-7b2a-4aab-ad66-7075fe67c5c7
📒 Files selected for processing (2)
apps/fleet-desktop-macos/FleetDesktop/FleetDesktopApp.swiftapps/fleet-desktop-macos/FleetDesktop/Info.plist
|
@allenhouchins tagging you in for a review if it's OK when you have time. I have not really changed the fleet desktop code itself, just the build process so we can pakcage up the SSO extension. This PR is just merging in your latest changes and bumping the version so that QA can start next week on this feature. I think with the upcoming release of v1.4.0 of fleet desktop we should be able to switch to publishing it from this repo for good |
|
@JordanMontgomery I just have one change to merge in in my repo — supporting the ability to link to the Install all button within a category. Will do my best to get that integrated this weekend then will code freeze until we build/release out of the Fleet repo. So we'll want to do one more upstream merge after that change goes through. |
|
@JordanMontgomery the upstream version of Fleet Desktop has been updated to 1.3.4. It includes quite a few bug fixes. Do you mind doing a new merge of that code and we can see where the code reviewers land? |
GitHub Actions
Inactive
Brings in the remaining work from allenhouchins/fleet-desktop (through v1.3.4, now frozen — this repo is the home of the code going forward): - fleet://install_all deep link with optional category_id, from v1.3.3 (the earlier partial merge only picked up FleetDesktopApp.swift) - v1.3.4: badge-count race fix for update_all/install_all deep links, WebView recovery after an expired SSO flow, security hardening - build.sh: build arm64/x86_64 in parallel - README: document the new deep links Kept fleet-side changes: PSSO extension build/signing support and the /var/run running-flag location (equivalent hardening to upstream's /var/db move). CFBundleVersion bumped to 10 so v1.4.0 sorts above upstream v1.3.4's build 9.
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/fleet-desktop-macos/FleetDesktop/BrowserWindow.swift`:
- Around line 515-519: Update the safeFilename validation near the
suggestedFilename handling to treat "." and ".." the same as an empty
lastPathComponent, replacing either with the existing "download" fallback before
appending it to downloadsDir. Preserve normal sanitized filenames unchanged.
In `@apps/fleet-desktop-macos/FleetDesktop/FleetService.swift`:
- Around line 323-331: Update categoryID(from:) so category_id validation
accepts only characters in the ASCII digit range 0–9, replacing the broader
Character.isNumber check. Preserve the existing non-empty requirement and nil
return for invalid values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 55e3b98d-b0d8-4436-ab37-c58a15c6c104
⛔ Files ignored due to path filters (1)
apps/fleet-desktop-macos/README.mdis excluded by!**/*.md
📒 Files selected for processing (4)
apps/fleet-desktop-macos/FleetDesktop/BrowserWindow.swiftapps/fleet-desktop-macos/FleetDesktop/FleetService.swiftapps/fleet-desktop-macos/FleetDesktop/Info.plistapps/fleet-desktop-macos/build.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/fleet-desktop-macos/FleetDesktop/Info.plist
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
apps/fleet-desktop-macos/FleetDesktop/FleetDesktopApp.swift:139
FleetService.setup()assumeshandleFleetURL()runs on the main thread (FleetService.swift:413-416).DistributedNotificationCenterdelivery thread isn’t guaranteed, so this can race with setup’s pending-state consumption. Also validate the forwarded URL’s scheme like the Apple Event handler does.
@objc private func handleForwardedURL(_ notification: Notification) {
guard let urlString = notification.userInfo?["url"] as? String,
let url = URL(string: urlString) else { return }
fleetService.handleFleetURL(url)
}
apps/fleet-desktop-macos/FleetDesktop/FleetDesktopApp.swift:143
- For consistency with other UI entry points and to avoid relying on
DistributedNotificationCenter’s delivery thread, dispatch this to the main queue before calling into the service.
@objc private func handleForwardedReopen(_ notification: Notification) {
fleetService.run()
}
|
@allenhouchins just reran the reviews. Let me know what you think of the latest round of comments |
… sync - Validate scheme and dispatch to main thread in forwarded-URL/reopen distributed notification handlers - Fix stale header comment about the token refresh timer - Restrict device token charset to explicit ASCII alphanumerics - Accept only ASCII digits for deep-link category_id - Treat '.'/'..' suggested download filenames as invalid - Collect both swiftc exit statuses in build.sh so a failed arch build doesn't leave the other running
URL(string:) accepts host-less values like "https://", which passed the scheme-only guard and later produced a device URL whose host was the literal segment "device", with the token in the path. Fail early with a clearer error instead.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
apps/fleet-desktop-macos/FleetDesktop/BrowserWindow.swift:76
- The new comments for
onWindowClose/onWindowShowsay they exist “so the timer can be paused/resumed”, but FleetService’s refresh timer is documented to run for the service lifetime (and this file doesn’t actually pause/resume any timer). This is misleading documentation; please revert to a generic callback description (or update to match real behavior).
/// Called when the window is closed (so the timer can be paused).
var onWindowClose: (() -> Void)?
/// Called when the window is shown (so the timer can be resumed).
var onWindowShow: (() -> Void)?
|
@JordanMontgomery this should be good to go! |
MagnusHJensen
left a comment
MagnusHJensen
left a comment
There was a problem hiding this comment.
Not fully in the loop on why the changes, but from an overall standpoint I think it looks reasonable
Related issue: Related to #45524
Checklist for submitter
If some of the following don't apply, delete the relevant line.
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Summary by CodeRabbit
Summary
New Features
fleet://install_allandfleet://install-alldeep links with optionalcategory_idto open the scoped self-service page and guide the install-all confirmation flow.Bug Fixes
Chores