Uh oh!
There was an error while loading. Please reload this page.
feat(macos): in-app updates from the macos-v* GitHub Releases - #495
Conversation
Pear-style updater for the menu bar app. AppUpdater checks the releases feed silently every 6 hours (signed installs only) and on demand from a new Updates section in Settings: Check for Updates, then Update Now to download the notarized zip, and Restart Now to swap the bundle and relaunch. Downloads are verified with codesign and pinned to the running app's signing team before install. The release pipeline now publishes a stapled BurnOSX-arm64.zip (the updater's download artifact) next to the DMG on both the macos-v* and macos-latest releases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W9WvBQecNXTrjL6vCgT8dh
📝 WalkthroughWalkthroughThis PR adds an in-app update mechanism to the macOS app via a new ChangesIn-app updater feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AppDelegate
participant AppUpdater
participant GitHubReleasesAPI
participant FileSystem
AppDelegate->>AppUpdater: startPeriodicChecks()
AppUpdater->>GitHubReleasesAPI: fetchLatestUpdate()
GitHubReleasesAPI-->>AppUpdater: release feed JSON
AppUpdater->>AppUpdater: compare versions, set phase available
AppUpdater->>GitHubReleasesAPI: downloadAndStage(zip)
GitHubReleasesAPI-->>AppUpdater: zip asset
AppUpdater->>FileSystem: extract, verify signature, stage bundle
AppUpdater->>FileSystem: swap installed bundle with staged bundle
AppUpdater->>AppUpdater: relaunch(appAt:)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:0162a5cf20
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try fm.copyItem(at: staged, to: installed) | ||
| } | ||
| } catch { | ||
| try? fm.moveItem(at: aside, to: installed) |
There was a problem hiding this comment.
If the copy fallback starts creating the new app bundle and then fails (for example due to disk-full or an interrupted cross-volume copy), installed may already exist as a partial bundle. The rollback move from aside back to installed will then fail because the destination exists, and the error is ignored, leaving the user with the old app removed and a broken partial install. Clear any partial destination before moving aside back so failed updates actually roll back.
Useful? React with 👍 / 👎.
Warning Gemini encountered an error creating the review. You can try again by commenting |
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/macos/Sources/Burn/AppUpdater.swift">
<violation number="1" location="apps/macos/Sources/Burn/AppUpdater.swift:299">
P2: The updater can install the wrong payload when a zip contains more than one `.app`, since it picks the first match without validating identity. Matching the extracted app to the running bundle identifier before staging would make payload selection deterministic and safer.</violation>
<violation number="2" location="apps/macos/Sources/Burn/AppUpdater.swift:340">
P1: A failed install can leave the app unrecoverable in-place because rollback suppresses errors and does not clear a partially created destination before restoring `aside`. Removing the destination before the rollback move makes recovery deterministic after mid-copy failures.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| try fm.copyItem(at: staged, to: installed) | ||
| } | ||
| } catch { | ||
| try? fm.moveItem(at: aside, to: installed) |
There was a problem hiding this comment.
P1: A failed install can leave the app unrecoverable in-place because rollback suppresses errors and does not clear a partially created destination before restoring aside. Removing the destination before the rollback move makes recovery deterministic after mid-copy failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/macos/Sources/Burn/AppUpdater.swift, line 340:
<comment>A failed install can leave the app unrecoverable in-place because rollback suppresses errors and does not clear a partially created destination before restoring `aside`. Removing the destination before the rollback move makes recovery deterministic after mid-copy failures.</comment>
<file context>
@@ -0,0 +1,407 @@
+ try fm.copyItem(at: staged, to: installed)
+ }
+ } catch {
+ try? fm.moveItem(at: aside, to: installed)
+ throw error
+ }
</file context>
| let contents = try FileManager.default.contentsOfDirectory( | ||
| at: extractDir, includingPropertiesForKeys: nil) | ||
| guard let app = contents.first(where: { $0.pathExtension == "app" }) else { |
There was a problem hiding this comment.
P2: The updater can install the wrong payload when a zip contains more than one .app, since it picks the first match without validating identity. Matching the extracted app to the running bundle identifier before staging would make payload selection deterministic and safer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/macos/Sources/Burn/AppUpdater.swift, line 299:
<comment>The updater can install the wrong payload when a zip contains more than one `.app`, since it picks the first match without validating identity. Matching the extracted app to the running bundle identifier before staging would make payload selection deterministic and safer.</comment>
<file context>
@@ -0,0 +1,407 @@
+
+ let contents = try FileManager.default.contentsOfDirectory(
+ at: extractDir, includingPropertiesForKeys: nil)
+ guard let app = contents.first(where: { $0.pathExtension == "app" }) else {
+ throw UpdateError.noAppInArchive
+ }
</file context>
Rebind self strongly before the timer's check task (capturing the weak var in concurrent code is an isolation error), and move the releases feed URL into the nonisolated fetch so it doesn't read a @mainactor static. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W9WvBQecNXTrjL6vCgT8dh
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/release-macos.yml (1)
110-118: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider hardening template interpolation flagged by zizmor.
--titleand--notes-fileinterpolate${{ }}expressions directly into therun:shell script. Static analysis flags this as a template-injection pattern; even thoughsteps.version.outputs.versionand the notes file path here are workflow-computed (low practical risk), passing them viaenv:and referencing$VARavoids the class of issue entirely.♻️ Proposed hardening
- name: Publish GitHub Release env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_VERSION: ${{ steps.version.outputs.version }}+ NOTES_FILE: ${{ steps.changelog.outputs.notes_file }} run: | TAG="${{ steps.version.outputs.tag }}" DMG="dist/BurnOSX-arm64.dmg" ZIP="dist/BurnOSX-arm64.zip" gh release create "${TAG}" "${DMG}" "${ZIP}" \ --repo "${GITHUB_REPOSITORY}" \ --target "${GITHUB_SHA}" \ - --title "Burn for Mac ${{ steps.version.outputs.version }}" \- --notes-file "${{ steps.changelog.outputs.notes_file }}"+ --title "Burn for Mac ${RELEASE_VERSION}" \+ --notes-file "${NOTES_FILE}"The ZIP attachment logic itself (versioned release + macos-latest pointer) is correct and matches the release.sh artifact and AppUpdater's expected asset name.
🤖 Prompt for 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. In @.github/workflows/release-macos.yml around lines 110 - 118, The release step in the macOS workflow directly interpolates workflow expressions into the shell script, which triggers template-injection hardening warnings. Move the values used by gh release create in the release job—especially the title and notes file path—into env variables, then reference those variables in the run command. Keep the existing release logic in the same gh release create and gh release delete block, but avoid using direct ${{ }} expressions inside the shell script.Source: Linters/SAST tools
apps/macos/Sources/Burn/ContentView.swift (1)
115-187: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSwitch is exhaustive and correctly wired to
AppUpdater's public API.
.idle/.checking/.upToDate/.available/.downloading/.readyToRestart/.failedall map 1:1 to the upstreamPhaseenum, and button actions (checkNow(),beginDownload(),restart()) match the corresponding phase-gated methods inAppUpdater.Minor: the
.upToDate,.available,.readyToRestart, and.failedcases repeat the sameVStack(alignment: .leading, spacing: 4) { Text(...); updateActionButton(...) }shape. Could be consolidated into a small helper to reduce duplication, but purely cosmetic given the modest size.♻️ Optional consolidation
+ private func updateMessageRow(+ _ message: String, color: Color = .primary,+ buttonTitle: String, systemImage: String, action: `@escaping` () -> Void+ ) -> some View {+ VStack(alignment: .leading, spacing: 4) {+ Text(message)+ .font(.caption)+ .foregroundStyle(color)+ .fixedSize(horizontal: false, vertical: true)+ updateActionButton(buttonTitle, systemImage: systemImage, action: action)+ }+ }🤖 Prompt for 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. In `@apps/macos/Sources/Burn/ContentView.swift` around lines 115 - 187, The updateControl switch is correct, but the .upToDate, .available, .readyToRestart, and .failed branches duplicate the same VStack/Text/button layout. Refactor ContentView by extracting that repeated structure into a small helper (for example alongside updateActionButton and updateProgressRow) that takes the message, button title, system image, and action, then use it from those Phase cases to keep the switch concise and easier to maintain.
🤖 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.
Nitpick comments:
In @.github/workflows/release-macos.yml:
- Around line 110-118: The release step in the macOS workflow directly
interpolates workflow expressions into the shell script, which triggers
template-injection hardening warnings. Move the values used by gh release create
in the release job—especially the title and notes file path—into env variables,
then reference those variables in the run command. Keep the existing release
logic in the same gh release create and gh release delete block, but avoid using
direct ${{ }} expressions inside the shell script.
In `@apps/macos/Sources/Burn/ContentView.swift`:
- Around line 115-187: The updateControl switch is correct, but the .upToDate,
.available, .readyToRestart, and .failed branches duplicate the same
VStack/Text/button layout. Refactor ContentView by extracting that repeated
structure into a small helper (for example alongside updateActionButton and
updateProgressRow) that takes the message, button title, system image, and
action, then use it from those Phase cases to keep the switch concise and easier
to maintain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f88b74f4-4fc2-4b86-a673-1d411eb16057
📒 Files selected for processing (7)
.github/workflows/release-macos.ymlapps/macos/README.mdapps/macos/Sources/Burn/AppDelegate.swiftapps/macos/Sources/Burn/AppUpdater.swiftapps/macos/Sources/Burn/ContentView.swiftapps/macos/Tests/BurnTests/AppUpdaterTests.swiftapps/macos/release.sh
Gives Burn for Mac the same release + auto-update experience as Pear. The release half already existed (
release-macos.ymlmirrors Pear's signing/notarization flow); this adds the in-app updater and its button.What's new
AppUpdater.swift— a native, dependency-free equivalent of Pear'selectron-updaterflow:macos-v*release carrying the zip asset — numeric version compare (2026.10.xbeats2026.9.x), drafts/prereleases/CLIrelayburn-v*tags skipped.codesign --verify, and pins the payload's team identifier to the running app's before install.Settings tab — new "Updates" section under Appearance: current version plus a phase-driven control (Check for Updates → Checking… → "Version X is available. Update Now" → Downloading… → Restart Now, with error + Try Again).
Release pipeline —
release.shnow packages a stapledBurnOSX-arm64.zip(the updater's download artifact) alongside the DMG, andrelease-macos.ymlattaches it to both the versionedmacos-v*release and themacos-latestpointer.Tests —
AppUpdaterTests.swiftcovers the feed-selection and version-comparison logic.Notes
macos-app-tests.ymlCI here rather than locally; worth a manual smoke test of the download → restart path on the first release cut after this merges.🤖 Generated with Claude Code
https://claude.ai/code/session_01W9WvBQecNXTrjL6vCgT8dh
Generated by Claude Code