Add Sparkle feed discovery and GitHub release integration #3
Conversation
- Add Anchoring + Decoy pricing in Pro tab ($3.99 → $2.99, decoy option)
- Implement Loss Aversion for security warnings ("Don't risk unpatched apps")
- Add Social Proof elements (user count, testimonials)
- Create Zeigarnik Effect progress indicators with completion tension
- Set smart defaults (auto-update ON) using Default Effect
- Enhance error states with Actor-Observer bias (situational framing)
- Add Authority signals (industry-standard verification)
- Improve Privacy tab with Reciprocity bias ("helps improve reliability")
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
- Add SecurityChecks for codesign verification and quarantine handling - Create Installer for ZIP/DMG/PKG with automatic backup before install - Implement Rollback system for one-click version restoration - Include cleanup utilities for managing backup storage - All installs verify signatures before replacing apps 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Implement SparkleHints for SUFeedURL discovery from app bundles - Add GitHub API client for releases with rate limiting and auth - Create RepoHints mapping for 30+ popular apps to GitHub repos - Add Brew cask integration for Homebrew-managed apps - Include repository validation and fallback discovery patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary of ChangesHello @adityash8, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the capabilities of the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality for discovering and installing application updates by integrating with Sparkle, GitHub Releases, and Homebrew. The overall structure is logical, with clear separation of concerns for handling different update sources and package types. However, my review identified several critical and high-severity issues that could impact the application's stability and reliability. These include potential crashes from force-unwrapped URLs, a critical bug in the backup restoration logic, brittle parsing of command-line tool output, and risky error handling that could lead to data loss or security vulnerabilities. I've provided detailed comments and suggestions to address these points.
| } | ||
|
|
||
| static func latest(owner: String, repo: String, token: String? = nil) async throws -> Release { | ||
| var request = URLRequest(url: URL(string: "https://api.github.com/repos/\(owner)/\(repo)/releases/latest")!) |
There was a problem hiding this comment.
Force-unwrapping a URL created from a string is unsafe and can lead to a runtime crash if the owner or repo strings contain characters that result in an invalid URL. You should safely unwrap the URL using a guard statement and throw an error if it's invalid.
guard let url = URL(string: "https://api.github.com/repos/\(owner)/\(repo)/releases/latest") else {
throw GitHubError.invalidRepo
}
var request = URLRequest(url: url)| } | ||
|
|
||
| static func releases(owner: String, repo: String, count: Int = 10, token: String? = nil) async throws -> [Release] { | ||
| var request = URLRequest(url: URL(string: "https://api.github.com/repos/\(owner)/\(repo)/releases?per_page=\(count)")!) |
There was a problem hiding this comment.
Force-unwrapping the URL with ! is dangerous as it can cause a runtime crash if the interpolated strings create an invalid URL. Please use safe URL construction, for example with a guard statement, to prevent potential crashes.
guard let url = URL(string: "https://api.github.com/repos/\(owner)/\(repo)/releases?per_page=\(count)") else {
throw GitHubError.invalidRepo
}
var request = URLRequest(url: url)| let dateA = (try? a.resourceValues(forKeys: [.creationDateKey]).creationDate) ?? .distantPast | ||
| let dateB = (try? b.resourceValues(forKeys: [.creationDateKey]).creationDate) ?? .distantPast | ||
| return dateA > dateB | ||
| }.first?.appendingPathComponent("\(bundleID).app") |
There was a problem hiding this comment.
There is a critical bug in how the backup path is reconstructed. The code assumes the application bundle is named \(bundleID).app, but the backup is created using the application's actual file name (e.g., MyAwesomeApp.app). This mismatch will cause latestBackup(bundleID:) to always fail to find the backup. You need to find the latest backup directory and then search for the .app file within it, regardless of its name.
| }.first?.appendingPathComponent("\(bundleID).app") | |
| }.first.flatMap { backupDirURL -> URL? in | |
| let contents = try? FileManager.default.contentsOfDirectory(at: backupDirURL, includingPropertiesForKeys: nil) | |
| return contents?.first { $0.pathExtension == "app" } | |
| } |
| guard let httpResponse = response as? HTTPURLResponse, | ||
| httpResponse.statusCode == 200 else { | ||
| throw URLError(.badServerResponse) | ||
| } |
There was a problem hiding this comment.
The error handling here is inconsistent with the latest function. This function throws a generic URLError(.badServerResponse) for any non-200 status code, whereas latest handles rate limiting (403) and other errors more specifically. It would be better to unify this logic to provide more specific GitHubError types, which will help with debugging and error reporting.
guard let httpResponse = response as? HTTPURLResponse else {
throw URLError(.badServerResponse)
}
if httpResponse.statusCode == 403 {
throw GitHubError.rateLimited
}
guard httpResponse.statusCode == 200 else {
throw GitHubError.apiError(httpResponse.statusCode)
}| // Create backup first | ||
| let currentAppPath = "/Applications/\(name).app" | ||
| if FileManager.default.fileExists(atPath: currentAppPath) { | ||
| _ = try? SecurityChecks.backup(appPath: currentAppPath, bundleID: bundleID, version: currentVersion) |
There was a problem hiding this comment.
Using _ = try? to handle the backup operation is risky. It silently ignores any errors during the backup process. If the backup fails and the subsequent installation also fails, the user could be left with no working version of the application. The backup failure should be handled explicitly, likely by throwing an error and aborting the update.
| _ = try? SecurityChecks.backup(appPath: currentAppPath, bundleID: bundleID, version: currentVersion) | |
| try SecurityChecks.backup(appPath: currentAppPath, bundleID: bundleID, version: currentVersion) |
| let task = Process() | ||
| task.executableURL = URL(fileURLWithPath: "/usr/bin/codesign") | ||
| task.arguments = ["--verify", "--deep", "--strict", appPath] | ||
| try? task.run() |
There was a problem hiding this comment.
Using try? task.run() is dangerous here. It suppresses errors that can occur when trying to launch the process, such as the codesign executable not being found. If task.run() throws, the task will not execute, but task.terminationStatus will remain 0, causing this function to incorrectly return true. This could lead to installing an unverified and potentially malicious application. The error should be caught and handled properly, or the function should be marked as throws.
| try? task.run() | |
| do { | |
| try task.run() | |
| } catch { | |
| print("Failed to run codesign: \(error)") | |
| return false | |
| } |
| struct CaskInfo: Decodable { | ||
| let token: String | ||
| let full_name: String | ||
| let tap: String | ||
| let version: String | ||
| let installed: String? | ||
| let outdated: Bool | ||
| let homepage: String? | ||
| let url: String | ||
| let name: [String] | ||
| let desc: String? | ||
| } |
There was a problem hiding this comment.
To adhere to Swift's API Design Guidelines, it's recommended to use camelCase for property names. You can use a CodingKeys enum to map the snake_case keys from the JSON payload to your camelCase properties. This improves consistency across your Swift codebase.
struct CaskInfo: Decodable {
let token: String
let fullName: String
let tap: String
let version: String
let installed: String?
let outdated: Bool
let homepage: String?
let url: String
let name: [String]
let desc: String?
enum CodingKeys: String, CodingKey {
case token, tap, version, installed, outdated, homepage, url, name, desc
case fullName = "full_name"
}
}| struct Release: Decodable { | ||
| let tag_name: String | ||
| let name: String? | ||
| let body: String? | ||
| let draft: Bool | ||
| let prerelease: Bool | ||
| let published_at: String? | ||
| let assets: [Asset] | ||
| } | ||
|
|
||
| struct Asset: Decodable { | ||
| let name: String | ||
| let browser_download_url: String | ||
| let content_type: String | ||
| let size: Int | ||
| } |
There was a problem hiding this comment.
For consistency with Swift API Design Guidelines, it's best to use camelCase for property names and map the snake_case keys from the GitHub API JSON response using a CodingKeys enum. This makes the Swift code more idiomatic.
struct Release: Decodable {
let tagName: String
let name: String?
let body: String?
let draft: Bool
let prerelease: Bool
let publishedAt: String?
let assets: [Asset]
enum CodingKeys: String, CodingKey {
case name, body, draft, prerelease, assets
case tagName = "tag_name"
case publishedAt = "published_at"
}
}
struct Asset: Decodable {
let name: String
let browserDownloadURL: String
let contentType: String
let size: Int
enum CodingKeys: String, CodingKey {
case name, size
case browserDownloadURL = "browser_download_url"
case contentType = "content_type"
}
}| } | ||
|
|
||
| private static func moveToApplications(_ src: URL) throws { | ||
| let dst = URL(fileURLWithPath: "/Applications").appendingPathComponent(src.lastPathComponent) |
There was a problem hiding this comment.
The /Applications path is hardcoded. While this is the default on macOS, it's not guaranteed. To make the code more robust and support non-standard configurations, you should retrieve the applications directory path dynamically using FileManager.default.urls(for: .applicationDirectory, in: .localDomainMask).
| let dst = URL(fileURLWithPath: "/Applications").appendingPathComponent(src.lastPathComponent) | |
| guard let applicationsURL = FileManager.default.urls(for: .applicationDirectory, in: .localDomainMask).first else { | |
| // This should ideally throw a specific error | |
| throw InstallerError.noAppFound | |
| } | |
| let dst = applicationsURL.appendingPathComponent(src.lastPathComponent) |
| for key in sparkleKeys { | ||
| if let value = bundle.object(forInfoDictionaryKey: key) { | ||
| print("Found Sparkle key \(key) for \(bundle.bundleIdentifier ?? "unknown"): \(value)") | ||
| } | ||
| } |
There was a problem hiding this comment.
This loop appears to be leftover debugging code. It iterates through a list of Sparkle keys and prints them if found, but it doesn't contribute to the function's goal of returning a feed URL. The function returns nil immediately after this block. This unused code should be removed to improve clarity.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } | ||
|
|
||
| static func track(_ name: String, props: [String: Any] = [:]) { | ||
| guard UserDefaults.standard.bool(forKey: "telemetry_enabled") else { return } |
There was a problem hiding this comment.
Bug: Telemetry Setting Mismatch
A key mismatch exists between the Telemetry service's UserDefaults check and the SettingsView's AppStorage key. The service looks for telemetry_enabled, but SettingsView uses telemetryEnabled. This prevents telemetry from respecting the user's setting, keeping it effectively disabled.
| } | ||
|
|
||
| static func latest(owner: String, repo: String, token: String? = nil) async throws -> Release { | ||
| var request = URLRequest(url: URL(string: "https://api.github.com/repos/\(owner)/\(repo)/releases/latest")!) |
There was a problem hiding this comment.
Bug: URL Initialization Crashes with Invalid Characters
The latest() and releases() functions force unwrap the URL(string:) initializer. If the owner or repo parameters contain characters that create an invalid URL string, this will cause the app to crash.
Additional Locations (2)
| let dateA = (try? a.resourceValues(forKeys: [.creationDateKey]).creationDate) ?? .distantPast | ||
| let dateB = (try? b.resourceValues(forKeys: [.creationDateKey]).creationDate) ?? .distantPast | ||
| return dateA > dateB | ||
| }.first?.appendingPathComponent("\(bundleID).app") |
There was a problem hiding this comment.
Bug: Backup Filename Mismatch Causes Rollback Failures
The latestBackup function constructs backup file paths using the bundle ID (e.g., bundleID.app). However, SecurityChecks.backup() stores these files using the original application's filename. This naming inconsistency means latestBackup can't find the correct backup, causing rollback operations to fail.
1 participant
Summary
Test plan
🤖 Generated with Claude Code