Skip to content

Add an in-app updater with Sparkle 2 - #260

Merged
sudoHG merged 3 commits into
mainfrom
sudoHG/issue-257-sparkle-updater-e31898
Nov 5, 2026
Merged

sudoHG merged 3 commits into
mainfrom
sudoHG/issue-257-sparkle-updater-e31898

Conversation

@sudoHG

@sudoHG sudoHG commented Nov 4, 2026

Copy link
Copy Markdown
Owner

Closes #257

Summary

  • Adds Sparkle 2.10.1 through SwiftPM (exact: "2.10.1", revision ab7d712) as a dependency of the production AskKeyApp executable only. AskKeyAppKit owns the SoftwareUpdater protocol, the observable SoftwareUpdateControls, and the pure SoftwareUpdateAvailability (enable) and SoftwareUpdateRelaunchPolicy (postpone) types. SparkleSoftwareUpdater in Sources/AskKeyApp/ wraps SPUStandardUpdaterController and SPUUpdaterDelegate. The production entry passes it as a factory through AppRuntimeConfiguration, and AppKit calls the factory only when the policy enables it.
  • Relaunches go through SoftwareUpdateRelaunchGate, which waits while any of these is present: a pending approval, an approved request the agent has not used yet, a running Broker call (runs with their deliveries, text, file and organization writes), an open upload, or a system authentication prompt. It rechecks every second and invokes Sparkle's handler exactly once when the app is idle. Timed allowances do not postpone the relaunch.
  • UI (only when the updater exists): Settings → General gets Check for Updates Automatically, plus an Updates row with Check for Updates… and a subtitle saying what a relaunch ends. The status item's right-click menu gets Check for Updates… above Quit Ask Key. Strings are in English and Simplified Chinese.
  • scripts/build-app.sh embeds Sparkle.framework in every mode and removes XPCServices. It makes sure the executable's rpath includes @executable_path/../Frameworks, signs nested code inside out without --deep, and writes the Sparkle keys only in Release mode. Release fails closed unless scripts/sparkle-public-key.txt holds one base64-encoded 32-byte key.
  • ADR 0011 records decisions 1–9. ADR 0005 and 0006 (superseded sentences only), features.md (row now Keep), architecture.md, both READMEs, SECURITY.md, CHANGELOG.md (Unreleased) and AGENTS.md → Product identifiers (feed URL) are updated.

Diff stat

git diff --stat origin/main: 33 files changed, 1356 insertions(+), 22 deletions(-) (21 modified, 12 new).

  • Modified: Package.swift, Package.resolved, Sources/AskKeyApp/AskKeyAppEntry.swift; AppKit AppDelegate.swift (+1), AppDelegate+BrokerLifecycle.swift (+9/−4), AppDelegate+Approval.swift (+1), AppDelegate+StatusItem.swift (+8/−2), AppRuntimeConfiguration.swift (+6/−1), VaultViewModel.swift (+2), FrozenSettingsPage.swift (+33), Localizable.xcstrings (+32); scripts/build-app.sh (+66/−2); docs.
  • New: Sources/AskKeyApp/SparkleSoftwareUpdater.swift; Sources/AskKeyAppKit/SoftwareUpdate/{SoftwareUpdateAvailability,SoftwareUpdateRelaunchPolicy,SoftwareUpdater,AgentActivityTracker}.swift; Sources/AskKeyAppKit/App/AppDelegate+SoftwareUpdate.swift; four Tests/AskKeyAppTests/SoftwareUpdate*Tests.swift; Tests/Automation/test_build_app_sparkle.py; docs/adr/0011-in-app-updates.md.
  • No change to Sources/AskKeyBroker/, Sources/AskKeyHelper/, Sources/AskKeyVault/, entitlements, workflows, site/, site-src/, package-release.sh or notarize-release.py. scripts/sparkle-public-key.txt is not added (Create the Sparkle update-signing key #258).

Tests

  • swift test --filter 'SoftwareUpdate': 25 passed, 0 failed, 0 skipped, in 4 new suites: SoftwareUpdateAvailabilityTests (8), SoftwareUpdateRelaunchPolicyTests (6), SoftwareUpdateActivityTrackerTests (7) and SoftwareUpdateControlsTests (4). None of these suites exist on main.
    • Enable policy: the release case; a missing, foreign, http or wrongly typed SUFeedURL; each testing flag set to true and to false; nil, dev, e2e and foreign bundle IDs; developmentAccepted, relocatedOrRenamed and helperMismatch; real OfficialInstallTopology.decide results; and a source scan confirming that only Sources/AskKeyApp/SparkleSoftwareUpdater.swift imports Sparkle.
    • Relaunch policy: idle; each busy state with its own reason; postpone then exactly one relaunch; a request that arrives while postponed (prompt answered → run starts → upload opens → idle); a repeated Sparkle request replacing the waiting one; and the run-loop timer.
    • Tracker: call counting, including after a throw; upload open, renew, freeze, cancel and expiry; and approvals counted while pending or approved, then forgotten.
  • python3 -m unittest discover -s Tests/Automation -v: 244 passed, 0 failed, 0 skipped. That is 233 on main plus 11 new tests in test_build_app_sparkle.py:
    • 4 script checks: feed URL matches the Swift constant and AGENTS.md; only Release writes the Sparkle keys, with the expected values; the framework is embedded in every mode without XPC services; signing order is inside out with no --deep signing.
    • 4 key-file checks run a stubbed copy of the script that stops right after the key check: Release fails without the file; Release rejects empty, non-base64, 31-byte, 33-byte, truncated and two-key files; Release accepts one valid key; Debug needs none.
    • 3 checks of the build-app.sh Debug bundle: no SU* keys, Sparkle embedded with no .xpc, codesign --verify --strict --deep passes, @executable_path/../Frameworks is present, and no rpath points into the checkout.
    • Locally the Debug-bundle checks ran against a fresh build-app.sh Debug output. In CI's build-and-test job no Debug bundle exists at the Automation step, so those 3 tests skip there (see Deviations).
  • The full swift test and the desktop flows run in CI. I did not run them locally.

Checks

Command Result
swift build Build complete
python3 scripts/check_hygiene.py passed (size=0 … cjk=0)
python3 scripts/check_module_deps.py passed, unchanged: product dependencies are outside its local-target graph
bash scripts/check_release_symbols.sh PASS: release app binary contains no forbidden E2E symbols or strings (after the fix in Deviations 3)
git grep -n 'import Sparkle' -- Sources | grep -v '^Sources/AskKeyApp/' no output
bash scripts/build-app.sh Debug succeeded (≥ 200 GiB free; app not launched)
PlistBuddy -c 'Print :SUFeedURL' …/Info.plist fails: Entry, ":SUFeedURL", Does Not Exist
test -d …/Contents/Frameworks/Sparkle.framework succeeds
find .build/AskKeyApp.app -name '*.xpc' | wc -l 0
codesign --verify --strict --deep .build/AskKeyApp.app succeeds
otool -l … | grep -A2 LC_RPATH | grep -c '@executable_path/../Frameworks' 1

Rpaths of the bundled Debug executable: /usr/lib/swift, @loader_path, @executable_path/../lib and @executable_path/../Frameworks. Xcode's absolute …/Build/Products/Debug/PackageFrameworks rpath is removed.

Decision 8: what a check sends, taken from the pinned Sparkle 2.10.1 source (.build/checkouts/Sparkle, tag 2.10.1, ab7d71291238df6e0492143b120c9b696a35f603), not from launching the app:

  • URL: GET https://askkey.sudohg.com/appcast.xml, with no query string. SUEnableSystemProfiling is NO, SUSendProfileInfo is unset and the delegate adds no feedParametersForUpdater, so parameterizedFeedURL returns the base URL (Sparkle/SPUUpdater.m:1249-1277). The check uses it at SPUUpdater.m:912 and SPUUpdater.m:921.
  • User-Agent: Sparkle's default is <name>/<CFBundleShortVersionString> Sparkle/2.10.1 (Sparkle/SPUUserAgent+Private.m:22; set at SPUUpdater.m:115). The name comes from the localized SUBundleName, CFBundleDisplayName or CFBundleName (Sparkle/SUHost.m:121-133; version at SUHost.m:165-172), and non-ASCII characters are dropped (SPUUserAgent+Private.m:23). That would turn 请旨 into ??, so the adapter sets userAgentString to Ask Key/<CFBundleShortVersionString> Sparkle/<Sparkle CFBundleShortVersionString>. It is applied to the request at Sparkle/SPUDownloadDriver.m:131-133.
  • Accept: application/rss+xml,*/*;q=0.1, on the appcast request only (Sparkle/SUAppcastDriver.m:74).
  • No other Sparkle headers: httpHeaders is unset and is merged at SPUDownloadDriver.m:137-142. The appcast bypasses the local cache (SPUDownloadDriver.m:129).
  • macOS URL loading adds its standard headers (Host, Accept-Language from the preferred languages, Accept-Encoding) and the default cookie storage, because Sparkle uses defaultSessionConfiguration (Downloader/SPUDownloader.m:84).
  • The archive download uses the same User-Agent and no Accept (Sparkle/SPUCoreBasedUpdateDriver.m:130). A sparkle:releaseNotesLink, if Publish a signed appcast with each release #259 uses one, is fetched with the same headers.

Deviations and questions

  1. Busy-state sources outside the listed AppKit files. The Broker and Vault expose no API for running calls or open uploads, and both are out of Scope. So the new AgentActivityTracker in AppKit counts calls: each Broker handler closure in AppDelegate+BrokerLifecycle.swift gets one beginCall/defer endCall line, and fileWrite is wrapped. Approved-but-unused requests are tracked by remembering tickets from pendingRequests() (one line in the pending-count observer in AppDelegate+Approval.swift, active only when the updater exists) and reading status(requestID:capability:). Both use existing public APIs only. Supporting one-liners: AppDelegate.swift (tracker property), VaultViewModel.swift (softwareUpdates property for Settings), and AppRuntimeConfiguration.swift (the makeSoftwareUpdater factory, which the E2E app does not pass).
  2. One extra busy state: "approved, not yet used." Without it, a relaunch between the user's approval and the agent's retry would drop the approval and the agent would have to ask again. Uploads count as open until they are frozen or cancelled, or for five minutes (the Broker's upload lifetime) after the last call on that upload. Component uploads stay open until that lifetime lapses.
  3. Enable policy details. The policy requires OfficialInstallDecision.accepted, not just allowsOfficialRuntime (which also allows developmentAccepted), and SUFeedURL must equal the product feed URL exactly. Testing flags are detected as "any Info.plist key with the AskKey prefix", whatever its value, rather than by listing the three names. Only Local (AskKeyLocalTesting), Debug (AskKeyRequiresDebugRunDirectory) and E2E (AskKeyE2ETesting) write such keys. Naming the E2E flag in production code put AskKeyE2E into the release binary and failed check_release_symbols.sh. The tests still cover all three names, plus a hypothetical later one.
  4. User-Agent override (see above), so the Chinese interface does not send ??.
  5. Rpath cleanup. Xcode links an absolute rpath into the build products (…/.derivedData/…/Build/Products/<config>/PackageFrameworks). It would ship a local path and let dyld find a build-tree Sparkle before the embedded one, so build-app.sh deletes rpaths into the checkout or derived data.
  6. Signing list. Contents/Helpers/AskKey_AskKeyHelper.bundle is now signed explicitly. Contents/Helpers is a nested-code location that --deep used to cover.
  7. UI details. The status menu has a separator between Check for Updates… and Quit Ask Key. The button row reuses the existing catalog key Updates (更新) as its title.
  8. CI coverage of the Debug-bundle checks. CI's build-and-test job runs the Automation suite without building a Debug bundle, and .github/workflows/ is out of Scope, so DebugBundleTests (3) skip there. They and the Acceptance commands ran locally against real output. basic-ui-flows runs build-app.sh E2E, which uses the same embedding and inside-out signing and ends with codesign --verify --strict --deep.
  9. Synthetic ASKKEY_* variables in one test. BuildAppReleaseKeyTests runs a stubbed copy of build-app.sh in a temporary directory, with synthetic ASKKEY_RELEASES_ENABLED, ASKKEY_CODESIGN_IDENTITY and ASKKEY_APPLE_TEAM_ID in the subprocess environment so it reaches the key check. Stubs stop it before any build or signing.
  10. Not done (out of Scope).
    • docs/README.md's ADR index is not updated; it also lacks 0009 and 0010.
    • Sparkle's "gentle reminders" for background apps (SPUStandardUserDriverDelegate.supportsGentleScheduledUpdateReminders) are not implemented. Sparkle may log a warning for this LSUIElement app when scheduled checks find an update.
    • Whether to embed release notes in the appcast or link them is left to Publish a signed appcast with each release #259.

Link Sparkle 2.10.1 into the production AskKeyApp executable only. AskKeyAppKit
defines the updater interface, the enable policy (official Release install
only) and the relaunch gate, which postpones Sparkle's relaunch while an
approval is pending or unused, a Broker call is running, an upload is open or
an authentication prompt is showing.

Settings > General and the status item menu show update controls only when
the updater exists. build-app.sh embeds Sparkle.framework without XPC
services in every mode, fixes the rpaths, signs nested code inside out without
--deep, and writes Sparkle keys only in Release, failing closed without
scripts/sparkle-public-key.txt.

ADR 0011 records the decisions, including the request headers an update check
sends.
@luoji-bot

luoji-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown

Review (planner, with an independent Codex GPT-6.1-Sol review): changes requested.

  1. Never accept an unsigned feed later. Sparkle 2.10.1 accepts a feed that fails signature validation once 20 days have passed since the first failure, unless SUSignedFeedFailureExpirationInterval is 0 (Sparkle/SUAppcastDriver.m:136-152; a zero interval disables that recovery). Add SUSignedFeedFailureExpirationInterval = 0 to the Release-only Info.plist keys, as a number Sparkle can read with doubleNumberForInfoDictionaryKey. Update ADR 0011 decision 2 and the Automation expectations to match.
  2. Gentle reminders for scheduled updates. When a scheduled check finds an update after launch, Sparkle shows the alert without activating a background app, and logs a warning that users may miss it (Sparkle/SPUStandardUserDriver.m:164-167, 272-329). Keep the change minimal:
    • Make SparkleSoftwareUpdater the SPUStandardUserDriverDelegate, and return true from supportsGentleScheduledUpdateReminders.
    • Return true from standardUserDriverShouldHandleShowingScheduledUpdate(_:andInImmediateFocus:), so Sparkle still shows the alert exactly as now and nothing steals focus.
    • In standardUserDriverWillHandleShowingUpdate(_:forUpdate:state:), when !state.userInitiated and notification authorization is already authorized or provisional, post one local notification with a fixed identifier that names the version. Do not request authorization, add a Dock or menu bar badge, or change the activation policy.
    • Add a softwareUpdate destination to NotificationRoute (only the route key). Its response calls checkForUpdates() and does not open the management window.
    • Remove the notification in standardUserDriverDidReceiveUserAttention(forUpdate:) and standardUserDriverWillFinishUpdateSession().
    • Strings in English and Simplified Chinese. Add tests for the route parsing (extra keys still fall back to .main) and for the post/no-post decision as a pure function. Add one sentence to ADR 0011.
  3. Optional copy fix. zh-Hans for the relaunch subtitle: 安装更新需要重启请旨,会等到没有待处理或进行中的请求时再重启。重启后请旨重新锁定,限时允许随之结束。 The English stays as it is.

Everything else covered by the review is fine as is: integrity settings, Release-only gating, the postpone selector match, XPC removal, inside-out signing, the rpath changes, the handler wrapping and the tracker's locking. Please make no other changes in this round.

- Release sets SUSignedFeedFailureExpirationInterval to 0, so Sparkle never
  accepts an unsigned feed after its 20-day failure window.
- Support Sparkle's gentle reminders for scheduled updates: Sparkle still
  shows its alert without taking focus, and when notifications are already
  authorized or provisional one local notification names the version.
  Clicking it brings the alert forward through a new softwareUpdate
  notification route without opening the management window; it is removed
  once the user attends to the update or the session ends.
- Shorten the Simplified Chinese relaunch subtitle.
- Mark the Chinese test literals for the hygiene check.
@luoji-bot

luoji-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown

Review round 1 addressed in 092590c.

  1. Unsigned feed never accepted later. Done. Release mode adds SUSignedFeedFailureExpirationInterval with PlistBuddy … real 0, which writes <real>0.0</real>. doubleNumberForInfoDictionaryKey reads that as zero, so fpclassify(...) == FP_ZERO turns off the recovery path (SUAppcastDriver.m:136-152). Decision 2 in ADR 0011 and the RELEASE_KEYS expectation in Tests/Automation/test_build_app_sparkle.py are updated to match.
  2. Gentle reminders. Done, keeping the change small.
    • SparkleSoftwareUpdater is now also the SPUStandardUserDriverDelegate (userDriverDelegate: self).
    • supportsGentleScheduledUpdateReminders returns true, and so does standardUserDriverShouldHandleShowingScheduledUpdate(_:andInImmediateFocus:).
    • standardUserDriverWillHandleShowingUpdate(_:forUpdate:state:) calls SoftwareUpdateReminder.post(version: update.displayVersionString, userInitiated: state.userInitiated). That function returns at once for user-initiated checks. Otherwise it reads the notification settings and adds a single request only when the status is .authorized or .provisional. The request uses the fixed identifier com.sudohg.askkey.software-update, has no trigger, and its body names the version. The reminder never requests authorization, never sets a badge and never changes the activation policy.
    • NotificationRoute.softwareUpdate maps to ["route": "softwareUpdate"] only; any extra key falls back to .main. ManagementNavigation.handleNotificationResponse handles this route before any page logic. A click calls checkForUpdates(), which AppDelegate passes as vault.softwareUpdates?.checkForUpdates(). It never calls openManagement and leaves the route and the notification revision as they were. A dismiss does nothing. Sparkle sets canCheckForUpdates back to YES once the alert is showing (SPUUpdater.m:893-897), and checkForUpdates then calls showUpdateInFocus (SPUUpdater.m:697-700).
    • The notification is removed in standardUserDriverDidReceiveUserAttention(forUpdate:) and in standardUserDriverWillFinishUpdateSession().
    • I type-checked all five selector names with #selector against the 2.10.1 headers. The delegate members are nonisolated because this protocol, unlike SPUUpdaterDelegate, has no main-actor annotation; they only call the thread-safe UNUserNotificationCenter helpers.
    • The copy is in English and Simplified Chinese: "Ask Key update available" / 请旨有新版本, and "Version %@ is available. Click to review it." / 版本 %@ 已推出,点击查看。
    • Tests: the new SoftwareUpdateReminderTests (3) cover the post/no-post decision as a pure function, the request contents (identifier, route, version, no trigger, no badge) and the copy. NotificationRouteTests gains the round trip, three fallback payloads and the click/dismiss behaviour. ADR 0011 decision 3 has one added sentence.
  3. Chinese relaunch subtitle. Done, using your wording exactly; the English is unchanged.

One change beyond the list: the hygiene cjk rule only scans tracked files. Once the first commit was in, the Chinese literals in SoftwareUpdateControlsTests.swift failed it, so the CI run for d110017 would have failed at that step, and the new reminder test had the same problem. Each of those lines now ends with the checker's existing // i18n-literal: marker. check_hygiene.py is unchanged.

Local results at 092590c:

Check Result
swift build Build complete
swift test --filter 'SoftwareUpdate|NotificationRoute' 32 passed, 0 failed, 0 skipped: NotificationRouteTests 4, SoftwareUpdateActivityTrackerTests 7, SoftwareUpdateAvailabilityTests 8, SoftwareUpdateControlsTests 4, SoftwareUpdateRelaunchPolicyTests 6, SoftwareUpdateReminderTests 3
python3 -m unittest discover -s Tests/Automation 244 passed, 0 failed, run against a fresh build-app.sh Debug bundle
python3 scripts/check_hygiene.py passed, with every file staged
python3 scripts/check_module_deps.py passed
bash scripts/check_release_symbols.sh PASS

@luoji-bot

luoji-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown

Review round 2 (planner, with Codex GPT-6.1-Sol): accepted, pending CI.

  • Item 1 is resolved. Sparkle reads the Release-only SUSignedFeedFailureExpirationInterval (real 0) as zero, which disables the unsigned-feed fallback (SUAppcastDriver.m:142).
  • Item 2 is resolved as specified. The delegate selectors match what Sparkle calls on the main thread, the alert's focus behaviour is unchanged, a reminder is posted only for scheduled checks with existing authorization, and the route opens only the update check.
  • Item 3 is applied.
  • Not fixing: Codex noted that a reminder could still be added if the user dismissed the alert before getNotificationSettings returned. Sparkle calls the delegate just before it shows the alert, and the settings callback returns in milliseconds, so this needs a sub-second human reaction. The worst outcome is one stale reminder that reopens the update check. That is not worth extra state.

No further changes requested. The PR can be merged once build-and-test and basic-ui-flows are green and the maintainer approves.

AskKeyApp now links @rpath/Sparkle.framework. The packaged authentication
tests copy the executable into a temporary Ask Key.app, where dyld could
no longer find the framework, so the child exited before reading stdin and
the second stdin write raised SIGPIPE in xctest on CI.

Copy Sparkle.framework from the same build products next to the copied
executable, where its @loader_path rpath resolves, and fail clearly when
the build products lack it.
@sudoHG
sudoHG merged commit 9a381d5 into main Nov 5, 2026
3 checks passed
@sudoHG
sudoHG deleted the sudoHG/issue-257-sparkle-updater-e31898 branch November 5, 2026 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an in-app updater with Sparkle 2

1 participant