Propagate errors in macOS FMA install scripts - #50198
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Fixes macOS Fleet-maintained app (FMA) install-script generation so install failures don’t get masked by a trailing relaunch_application that exits 0, ensuring Fleet reports failed installs as failures.
Changes:
- Update the Homebrew macOS install-script generator to propagate failures from
installer -pkgand from the finalcp -Rapp install step (including restore-on-failure behavior for thecp -Rpath). - Remove the now-unused
scriptBuilder.Copyhelper. - Add generator tests covering pkg, pkg-with-choices, and app copy failure-propagation behavior.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ee/maintained-apps/ingesters/homebrew/scripts.go | Adjusts generated bash to exit non-zero on install failures and restore apps on failed copy installs. |
| ee/maintained-apps/ingesters/homebrew/scripts_test.go | Adds regression tests asserting failure propagation / restore snippets are present in generated scripts. |
| changes/50056-fma-install-scripts-ignore-errors | User-visible change note (content excluded from review). |
Files excluded by content exclusion policy (1)
- changes/50056-fma-install-scripts-ignore-errors
Comments suppressed due to low confidence (1)
ee/maintained-apps/ingesters/homebrew/scripts.go:58
- The
cp -Rfailure path always exits with status 1, which loses the real exit code fromcp/sudo. If this fails in the field, preserving the original non-zero status can make diagnosing failures easier (and avoids masking distinctions like permission vs. missing source).
sudo rm -rf "$APPDIR/%[1]s"
sudo mv "$TMPDIR/%[1]s.bkp" "$APPDIR/%[1]s"
fi
exit 1
fi`, appPath)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50198 +/- ##
==========================================
+ Coverage 68.08% 68.10% +0.02%
==========================================
Files 3936 3936
Lines 250690 250927 +237
Branches 13437 13437
==========================================
+ Hits 170687 170905 +218
+ Misses 64690 64687 -3
- Partials 15313 15335 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/50056-fma-install-scripts-ignore-errors
Comments suppressed due to low confidence (1)
ee/maintained-apps/ingesters/homebrew/scripts_test.go:99
- This assertion is too broad:
"exit 1"will also match the DMG extraction guard (... || exit 1), so this test could pass even if the new copy-failure/restore block (with its ownexit 1) is missing. Make the match specific to the restore block (e.g. include the followingfi).
require.Contains(t, script, `sudo mv "$APPDIR/Foo.app" "$TMPDIR/Foo.app.bkp" || exit $?`)
require.Contains(t, script, `if ! sudo cp -R "$TMPDIR/Foo.app" "$APPDIR"; then`)
require.Contains(t, script, `sudo mv "$TMPDIR/Foo.app.bkp" "$APPDIR/Foo.app"`)
require.Contains(t, script, "exit 1")
}
cdcme
marked this pull request as ready for review
July 30, 2026 12:27
Contributor
WalkthroughGenerated macOS install scripts now propagate failures from package installers, extraction, mounting, and application-copy operations. Existing applications are backed up conditionally, partial copies are removed, and previous versions are restored after failed replacements. Published app metadata references updated scripts, with tests covering package installs, choice changes, quoted paths, and application rollback. The changelog identifies the affected script categories. Possibly related issues
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@ee/maintained-apps/ingesters/homebrew/scripts_test.go`:
- Around line 50-51: Strengthen the assertions in
ee/maintained-apps/ingesters/homebrew/scripts_test.go at lines 50-51, 72-74, and
94-98: assert relaunch_application follows the guarded installer command;
validate one complete choices-based installer command using a pkg filename with
spaces; and assert partial-target removal, restoration, then exit 1 occur in
order within the failed-copy branch, using index comparisons or complete
command-block matching rather than independent fragments.
In `@ee/maintained-apps/ingesters/homebrew/scripts.go`:
- Around line 48-57: Update the replacement script generated by the app-install
flow around the appPath backup logic to use a fresh, non-existing backup path
for every replacement. Ensure the same unique path is used by the failure
restore branch, preventing mv from nesting an existing backup and restoring the
original app layout correctly.
🪄 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 Plus
Run ID: 38a36a0b-5b81-46e8-9d33-82134f9ea194
📒 Files selected for processing (3)
changes/50056-fma-install-scripts-ignore-errorsee/maintained-apps/ingesters/homebrew/scripts.goee/maintained-apps/ingesters/homebrew/scripts_test.go
Comment on lines
+50
to
+51
| // relaunch still runs, but only after the install command's exit code is checked. | ||
| require.Contains(t, script, "relaunch_application 'com.example.Foo'") |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the failure-flow structure, not independent fragments.
These checks can all pass if commands are reordered or emitted outside the intended failure branch. Compare indices or assert complete command blocks.
ee/maintained-apps/ingesters/homebrew/scripts_test.go#L50-L51: assertrelaunch_applicationoccurs after the guarded installer command.ee/maintained-apps/ingesters/homebrew/scripts_test.go#L72-L74: assert one complete choices-based installer command, using a pkg filename containing spaces.ee/maintained-apps/ingesters/homebrew/scripts_test.go#L94-L98: assert partial-target removal, restoration, andexit 1occur in that order inside the failed-copy branch.
📍 Affects 1 file
ee/maintained-apps/ingesters/homebrew/scripts_test.go#L50-L51(this comment)ee/maintained-apps/ingesters/homebrew/scripts_test.go#L72-L74ee/maintained-apps/ingesters/homebrew/scripts_test.go#L94-L98
🤖 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 `@ee/maintained-apps/ingesters/homebrew/scripts_test.go` around lines 50 - 51,
Strengthen the assertions in
ee/maintained-apps/ingesters/homebrew/scripts_test.go at lines 50-51, 72-74, and
94-98: assert relaunch_application follows the guarded installer command;
validate one complete choices-based installer command using a pkg filename with
spaces; and assert partial-target removal, restoration, then exit 1 occur in
order within the failed-copy branch, using index comparisons or complete
command-block matching rather than independent fragments.
Comment on lines
48
to
+57
| sb.Writef(`if [ -d "$APPDIR/%[1]s" ]; then | ||
| sudo mv "$APPDIR/%[1]s" "$TMPDIR/%[1]s.bkp" | ||
| sudo mv "$APPDIR/%[1]s" "$TMPDIR/%[1]s.bkp" || exit $? | ||
| fi`, appPath) | ||
| sb.Writef(`if ! sudo cp -R "$TMPDIR/%[1]s" "$APPDIR"; then | ||
| # restore the previous version so a failed install doesn't leave the host with nothing | ||
| if [ -d "$TMPDIR/%[1]s.bkp" ]; then | ||
| sudo rm -rf "$APPDIR/%[1]s" | ||
| sudo mv "$TMPDIR/%[1]s.bkp" "$APPDIR/%[1]s" | ||
| fi | ||
| exit 1 |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use a fresh backup path for each replacement.
The successful path leaves "$TMPDIR/<app>.bkp" behind. Reusing that TMPDIR makes mv nest the existing app inside the prior backup; a later restore then recreates /Applications/<app> as a nested directory instead of the original app. Generate a unique backup path (or safely clean a controlled one) and use it consistently for restore.
🤖 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 `@ee/maintained-apps/ingesters/homebrew/scripts.go` around lines 48 - 57,
Update the replacement script generated by the app-install flow around the
appPath backup logic to use a fresh, non-existing backup path for every
replacement. Ensure the same unique path is used by the failure restore branch,
preventing mv from nesting an existing backup and restoring the original app
layout correctly.
Contributor
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/50056-fma-install-scripts-ignore-errors
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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 `@ee/maintained-apps/inputs/homebrew/scripts/grammarly-desktop-install.sh`:
- Around line 74-79: Update the Grammarly installation flow around the sudo cp
failure branch to move the existing Grammarly Desktop.app to a backup location
before replacing it, then restore that backup after removing the partial copy
when installation fails. Preserve the current failure message and exit behavior,
and clean up the backup after a successful replacement.
In `@ee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.sh`:
- Around line 92-97: Update the failed-copy branch of the Microsoft Edge
installation flow to restore $TMPDIR/Microsoft Edge.app.bkp back to
$APPDIR/Microsoft Edge.app after removing the partial copy and before exiting.
Preserve the existing failure message and exit status, and only perform the
restore when the backup exists.
In `@ee/maintained-apps/outputs/adobe-acrobat-pro/darwin.json`:
- Line 19: Guard each archive extraction before proceeding: in
ee/maintained-apps/outputs/adobe-acrobat-pro/darwin.json at lines 19-19, check
the DMG copy and detach operations and exit on failure; in
ee/maintained-apps/outputs/comet/darwin.json at lines 19-19 and
ee/maintained-apps/outputs/evernote/darwin.json at lines 19-19, check the DMG
copy before moving or replacing the existing application; in
ee/maintained-apps/outputs/wins/darwin.json at lines 20-20, check unzip before
copying Wins.app.
In `@ee/maintained-apps/outputs/firealpaca/darwin.json`:
- Line 19: Update the failed-copy rollback in the generated FireAlpaca installer
script so the backup restoration command is checked explicitly. If moving
FireAlpaca.app.bkp back to APPDIR fails, report the restoration error and exit
nonzero rather than continuing as though rollback succeeded; preserve the
existing cleanup and successful restoration behavior.
- Line 19: Make rollback restoration mandatory and checked in the installation
failure paths: in ee/maintained-apps/outputs/firealpaca/darwin.json:19-19,
ee/maintained-apps/outputs/github/darwin.json:20-20,
ee/maintained-apps/outputs/keeper-password-manager/darwin.json:20-20,
ee/maintained-apps/outputs/nvidia-geforce-now/darwin.json:20-20, and
ee/maintained-apps/outputs/pd/darwin.json:19-19, check the backup-app mv
operation and fail explicitly if restoration fails. In
ee/maintained-apps/outputs/microsoft-edge/darwin.json:19-20, create a per-run
backup before replacement and check its restoration in the same failure path;
preserve removal of the partial replacement before attempting restoration.
In `@ee/maintained-apps/outputs/grammarly-desktop/darwin.json`:
- Line 20: Update the install flow around quit_application and the existing
Grammarly Desktop.app removal to move the current app to a temporary backup
instead of deleting it. If copying the replacement fails, remove the partial app
and restore the backup; delete the backup only after a successful copy,
preserving the existing behavior when no prior app exists.
In `@ee/maintained-apps/outputs/keeper-password-manager/darwin.json`:
- Line 20: The failure path in the installation block must verify restoration of
“Keeper Password Manager.app.bkp” after a failed replacement copy. Update the
rollback logic following the failed `sudo cp -R` so a failed `sudo mv`
restoration is detected and surfaced with an error message and nonzero exit
status, while preserving the existing cleanup and successful restoration
behavior.
In `@ee/maintained-apps/outputs/nvidia-geforce-now/darwin.json`:
- Line 20: Update the failed-copy rollback block after the sudo cp command to
check the status of restoring GeForceNOW.app.bkp via sudo mv. If restoration
fails, report the failure and exit nonzero; otherwise preserve the existing
rollback and exit behavior.
In `@ee/maintained-apps/outputs/p4v/darwin.json`:
- Line 19: Update the installation loop in the generated script to validate that
the required p4v.app bundle exists at MOUNT_POINT before copying applications.
If it is missing, detach the mounted DMG, print an installation failure message,
and exit nonzero; preserve the existing handling for optional p4merge.app and
p4admin.app bundles.
- Line 19: Update the installation loop for p4v.app, p4merge.app, and
p4admin.app to back up each existing application bundle before removing it, then
restore that backup if ditto fails. Preserve the existing cleanup of partial
copies and ensure successful installations remove the backup only after the
replacement completes.
In `@ee/maintained-apps/outputs/pd/darwin.json`:
- Line 19: Update the failed-copy rollback block around the sudo mv command
restoring $TMPDIR/$APP_NAME.bkp so its failure is checked and propagated instead
of ignored. Preserve the existing cleanup and exit behavior, but ensure the
script exits unsuccessfully when restoration of the previous Pd bundle fails.
In `@ee/maintained-apps/outputs/pritunl/darwin.json`:
- Line 20: Update the extraction flow before quit_and_track_application and
installer so unzip failure immediately exits with its status, then verify the
expected Pritunl.pkg exists in the current TMPDIR extraction output before
invoking installer. Do not proceed to installation or relaunch when extraction
fails or the package is missing.
🪄 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 Plus
Run ID: 484f55b9-eeca-4715-94fc-202c9dd72f77
📒 Files selected for processing (31)
changes/50056-fma-install-scripts-ignore-errorsee/maintained-apps/ingesters/homebrew/scripts.goee/maintained-apps/ingesters/homebrew/scripts_test.goee/maintained-apps/inputs/homebrew/scripts/cycling74-max-install.shee/maintained-apps/inputs/homebrew/scripts/github-desktop-install.shee/maintained-apps/inputs/homebrew/scripts/google_chrome_install.shee/maintained-apps/inputs/homebrew/scripts/grammarly-desktop-install.shee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.shee/maintained-apps/inputs/homebrew/scripts/p4v-install.shee/maintained-apps/inputs/homebrew/scripts/pd-install.shee/maintained-apps/inputs/homebrew/scripts/webex_install.shee/maintained-apps/inputs/homebrew/scripts/zoom_install.shee/maintained-apps/outputs/adobe-acrobat-pro/darwin.jsonee/maintained-apps/outputs/comet/darwin.jsonee/maintained-apps/outputs/cycling74-max/darwin.jsonee/maintained-apps/outputs/evernote/darwin.jsonee/maintained-apps/outputs/firealpaca/darwin.jsonee/maintained-apps/outputs/github/darwin.jsonee/maintained-apps/outputs/google-chrome/darwin.jsonee/maintained-apps/outputs/grammarly-desktop/darwin.jsonee/maintained-apps/outputs/keeper-password-manager/darwin.jsonee/maintained-apps/outputs/microsoft-edge/darwin.jsonee/maintained-apps/outputs/nvidia-geforce-now/darwin.jsonee/maintained-apps/outputs/p4v/darwin.jsonee/maintained-apps/outputs/pd/darwin.jsonee/maintained-apps/outputs/pritunl/darwin.jsonee/maintained-apps/outputs/vnc-viewer/darwin.jsonee/maintained-apps/outputs/webex/darwin.jsonee/maintained-apps/outputs/wins/darwin.jsonee/maintained-apps/outputs/worksheet-crafter/darwin.jsonee/maintained-apps/outputs/zoom/darwin.json
🚧 Files skipped from review as they are similar to previous changes (1)
- changes/50056-fma-install-scripts-ignore-errors
Member
|
@cdcme thanks for catching this! I updated all the custom install scripts and frozen app scripts to replicate this new behavior too since those don't go through the scripts generator. Once this has |
allenhouchins
approved these changes
Jul 30, 2026
Contributor
Script Diff Resultsee/maintained-apps/outputs/adobe-acrobat-pro/darwin.json=== Install // 7bd42f17 -> afddc3a7 ===
--- /tmp/old.ZaBUav 2026-07-30 17:04:20.189421407 +0000
+++ /tmp/new.5HxFtk 2026-07-30 17:04:20.189421407 +0000
@@ -101,5 +101,5 @@
hdiutil detach "$MOUNT_POINT" || true
# install pkg files
quit_and_track_application 'com.adobe.Acrobat.Pro'
-sudo installer -pkg "$TMPDIR/Acrobat/Acrobat DC Installer.pkg" -target /
+sudo installer -pkg "$TMPDIR/Acrobat/Acrobat DC Installer.pkg" -target / || exit $?
relaunch_application 'com.adobe.Acrobat.Pro'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/comet/darwin.json=== Install // af8d196c -> 702b27b8 ===
--- /tmp/old.tLUwKN 2026-07-30 17:04:20.256421745 +0000
+++ /tmp/new.K0ZpW3 2026-07-30 17:04:20.256421745 +0000
@@ -102,7 +102,15 @@
# copy to the applications folder
quit_and_track_application 'ai.perplexity.comet'
if [ -d "$APPDIR/Comet.app" ]; then
- sudo mv "$APPDIR/Comet.app" "$TMPDIR/Comet.app.bkp"
+ sudo mv "$APPDIR/Comet.app" "$TMPDIR/Comet.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/Comet.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/Comet.app"
+ if [ -d "$TMPDIR/Comet.app.bkp" ]; then
+ sudo mv "$TMPDIR/Comet.app.bkp" "$APPDIR/Comet.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/Comet.app" "$APPDIR"
relaunch_application 'ai.perplexity.comet'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/cycling74-max/darwin.json=== Install // b553ba8e -> 3c5d2cd6 ===
--- /tmp/old.EcKMVr 2026-07-30 17:04:20.312422027 +0000
+++ /tmp/new.5VZFaA 2026-07-30 17:04:20.312422027 +0000
@@ -101,12 +101,23 @@
# DMG never mounts, and nothing gets installed.
MOUNT_POINT=$(mktemp -d /tmp/dmg_mount_XXXXXX)
yes | hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH" || exit 1
-sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"
+if ! sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"; then
+ hdiutil detach "$MOUNT_POINT" || true
+ exit 1
+fi
hdiutil detach "$MOUNT_POINT" || true
# copy to the applications folder
quit_and_track_application 'com.cycling74.Max'
if [ -d "$APPDIR/Max.app" ]; then
- sudo mv "$APPDIR/Max.app" "$TMPDIR/Max.app.bkp"
+ sudo mv "$APPDIR/Max.app" "$TMPDIR/Max.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/Max.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/Max.app"
+ if [ -d "$TMPDIR/Max.app.bkp" ]; then
+ sudo mv "$TMPDIR/Max.app.bkp" "$APPDIR/Max.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/Max.app" "$APPDIR"
relaunch_application 'com.cycling74.Max'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/evernote/darwin.json=== Install // 67a65f7d -> 4cd3103c ===
--- /tmp/old.J8LPa6 2026-07-30 17:04:20.388422410 +0000
+++ /tmp/new.GHEKgr 2026-07-30 17:04:20.388422410 +0000
@@ -102,7 +102,15 @@
# copy to the applications folder
quit_and_track_application 'com.evernote.Evernote'
if [ -d "$APPDIR/Evernote.app" ]; then
- sudo mv "$APPDIR/Evernote.app" "$TMPDIR/Evernote.app.bkp"
+ sudo mv "$APPDIR/Evernote.app" "$TMPDIR/Evernote.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/Evernote.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/Evernote.app"
+ if [ -d "$TMPDIR/Evernote.app.bkp" ]; then
+ sudo mv "$TMPDIR/Evernote.app.bkp" "$APPDIR/Evernote.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/Evernote.app" "$APPDIR"
relaunch_application 'com.evernote.Evernote'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/firealpaca/darwin.json=== Install // f2cad371 -> a03f6a1f ===
--- /tmp/old.pYPKpv 2026-07-30 17:04:20.458422762 +0000
+++ /tmp/new.a1P4AG 2026-07-30 17:04:20.458422762 +0000
@@ -99,7 +99,15 @@
# copy to the applications folder
quit_and_track_application 'com.firealpaca'
if [ -d "$APPDIR/FireAlpaca.app" ]; then
- sudo mv "$APPDIR/FireAlpaca.app" "$TMPDIR/FireAlpaca.app.bkp"
+ sudo mv "$APPDIR/FireAlpaca.app" "$TMPDIR/FireAlpaca.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/FireAlpaca.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/FireAlpaca.app"
+ if [ -d "$TMPDIR/FireAlpaca.app.bkp" ]; then
+ sudo mv "$TMPDIR/FireAlpaca.app.bkp" "$APPDIR/FireAlpaca.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/FireAlpaca.app" "$APPDIR"
relaunch_application 'com.firealpaca'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/github/darwin.json=== Install // c91ea2b5 -> 7ed6a869 ===
--- /tmp/old.ZjQdVy 2026-07-30 17:04:20.511423029 +0000
+++ /tmp/new.wkgh91 2026-07-30 17:04:20.511423029 +0000
@@ -94,14 +94,22 @@
}
# Extract with ditto and --noqtn so extracted files do NOT get quarantine.
-ditto -xk --noqtn "$INSTALLER_PATH" "$TMPDIR"
+ditto -xk --noqtn "$INSTALLER_PATH" "$TMPDIR" || exit $?
# copy to the applications folder (do not modify the app bundle after extraction)
quit_and_track_application 'com.github.GitHubClient'
if [ -d "$APPDIR/GitHub Desktop.app" ]; then
- sudo mv "$APPDIR/GitHub Desktop.app" "$TMPDIR/GitHub Desktop.app.bkp"
+ sudo mv "$APPDIR/GitHub Desktop.app" "$TMPDIR/GitHub Desktop.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/GitHub Desktop.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/GitHub Desktop.app"
+ if [ -d "$TMPDIR/GitHub Desktop.app.bkp" ]; then
+ sudo mv "$TMPDIR/GitHub Desktop.app.bkp" "$APPDIR/GitHub Desktop.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/GitHub Desktop.app" "$APPDIR"
relaunch_application 'com.github.GitHubClient'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/google-chrome/darwin.json=== Install Script (no changes) ===
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/grammarly-desktop/darwin.json=== Install // df3d0525 -> 8274b7dd ===
--- /tmp/old.rVo5ho 2026-07-30 17:04:20.591423432 +0000
+++ /tmp/new.cNJejc 2026-07-30 17:04:20.591423432 +0000
@@ -46,9 +46,17 @@
}
# extract contents
+# Fail before the existing app is removed below, so a bad download can't leave
+# the host without a working install.
MOUNT_POINT=$(mktemp -d /tmp/dmg_mount_XXXXXX)
-hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH"
-sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"
+if ! hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH"; then
+ echo "Failed to mount DMG '$INSTALLER_PATH'." >&2
+ exit 1
+fi
+if ! sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"; then
+ hdiutil detach "$MOUNT_POINT" || true
+ exit 1
+fi
hdiutil detach "$MOUNT_POINT"
# copy to the applications folder
@@ -63,7 +71,12 @@
# Copy Grammarly Installer.app from temp directory to Applications as Grammarly Desktop.app
if [ -d "$TMPDIR/Grammarly Installer.app" ]; then
- sudo cp -R "$TMPDIR/Grammarly Installer.app" "$APPDIR/Grammarly Desktop.app"
+ if ! sudo cp -R "$TMPDIR/Grammarly Installer.app" "$APPDIR/Grammarly Desktop.app"; then
+ # remove the partial copy so a failed install isn't inventoried as the new version
+ sudo rm -rf "$APPDIR/Grammarly Desktop.app"
+ echo "Installation failed"
+ exit 1
+ fi
echo "Installation verified"
else
echo "Error: Grammarly Installer.app not found in extracted files"
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/keeper-password-manager/darwin.json=== Install // 7b4bffee -> bd9c8589 ===
--- /tmp/old.ISvFA1 2026-07-30 17:04:20.660423780 +0000
+++ /tmp/new.EafrXR 2026-07-30 17:04:20.661423785 +0000
@@ -102,7 +102,15 @@
# copy to the applications folder
quit_and_track_application 'com.keepersecurity.passwordmanager'
if [ -d "$APPDIR/Keeper Password Manager.app" ]; then
- sudo mv "$APPDIR/Keeper Password Manager.app" "$TMPDIR/Keeper Password Manager.app.bkp"
+ sudo mv "$APPDIR/Keeper Password Manager.app" "$TMPDIR/Keeper Password Manager.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/Keeper Password Manager.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/Keeper Password Manager.app"
+ if [ -d "$TMPDIR/Keeper Password Manager.app.bkp" ]; then
+ sudo mv "$TMPDIR/Keeper Password Manager.app.bkp" "$APPDIR/Keeper Password Manager.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/Keeper Password Manager.app" "$APPDIR"
relaunch_application 'com.keepersecurity.passwordmanager'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/microsoft-edge/darwin.json=== Install // 81ff19c2 -> 4c051c40 ===
--- /tmp/old.abnxaj 2026-07-30 17:04:20.709424027 +0000
+++ /tmp/new.tuyFl9 2026-07-30 17:04:20.709424027 +0000
@@ -46,9 +46,17 @@
}
# extract contents
+# Fail before the existing app is removed below, so a bad download can't leave
+# the host without a working install.
MOUNT_POINT=$(mktemp -d /tmp/dmg_mount_XXXXXX)
-hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH"
-sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"
+if ! hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH"; then
+ echo "Failed to mount DMG '$INSTALLER_PATH'." >&2
+ exit 1
+fi
+if ! sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"; then
+ hdiutil detach "$MOUNT_POINT" || true
+ exit 1
+fi
hdiutil detach "$MOUNT_POINT"
# Clean up any backup files that might exist from previous failed installations
@@ -81,7 +89,12 @@
fi
# Install the new app
-sudo cp -R "$TMPDIR/Microsoft Edge.app" "$APPDIR"
+if ! sudo cp -R "$TMPDIR/Microsoft Edge.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new version
+ sudo rm -rf "$APPDIR/Microsoft Edge.app"
+ echo "Installation failed"
+ exit 1
+fi
# Verify installation and do final cleanup
if [ -d "$APPDIR/Microsoft Edge.app" ]; then
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/nvidia-geforce-now/darwin.json=== Install // a9979922 -> 88b8bf1c ===
--- /tmp/old.gb0qtR 2026-07-30 17:04:20.778424375 +0000
+++ /tmp/new.GPNWrF 2026-07-30 17:04:20.778424375 +0000
@@ -102,7 +102,15 @@
# copy to the applications folder
quit_and_track_application 'com.nvidia.gfnpc.mall'
if [ -d "$APPDIR/GeForceNOW.app" ]; then
- sudo mv "$APPDIR/GeForceNOW.app" "$TMPDIR/GeForceNOW.app.bkp"
+ sudo mv "$APPDIR/GeForceNOW.app" "$TMPDIR/GeForceNOW.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/GeForceNOW.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/GeForceNOW.app"
+ if [ -d "$TMPDIR/GeForceNOW.app.bkp" ]; then
+ sudo mv "$TMPDIR/GeForceNOW.app.bkp" "$APPDIR/GeForceNOW.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/GeForceNOW.app" "$APPDIR"
relaunch_application 'com.nvidia.gfnpc.mall'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/p4v/darwin.json=== Install // 7f44eda5 -> 3604c875 ===
--- /tmp/old.j6EV2x 2026-07-30 17:04:20.837424672 +0000
+++ /tmp/new.DfoWCG 2026-07-30 17:04:20.837424672 +0000
@@ -31,13 +31,24 @@
for app in p4v.app p4merge.app p4admin.app; do
if [[ -d "$MOUNT_POINT/$app" ]]; then
rm -rf "$APPDIR/$app" >/dev/null 2>&1 || true
- ditto "$MOUNT_POINT/$app" "$APPDIR/$app" >/dev/null 2>&1
+ if ! ditto "$MOUNT_POINT/$app" "$APPDIR/$app"; then
+ # remove the partial copy so a failed install isn't inventoried as installed
+ rm -rf "$APPDIR/$app" >/dev/null 2>&1 || true
+ hdiutil detach "$MOUNT_POINT" >/dev/null 2>&1 || true
+ echo "failed to install $app"
+ exit 1
+ fi
fi
done
# Install p4vc command line binary to /usr/local/bin
if [[ -f "$MOUNT_POINT/p4vc" ]]; then
- cp "$MOUNT_POINT/p4vc" /usr/local/bin/p4vc
+ mkdir -p /usr/local/bin
+ if ! cp "$MOUNT_POINT/p4vc" /usr/local/bin/p4vc; then
+ hdiutil detach "$MOUNT_POINT" >/dev/null 2>&1 || true
+ echo "failed to install p4vc"
+ exit 1
+ fi
chmod +x /usr/local/bin/p4vc
chown root:wheel /usr/local/bin/p4vc
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/pd/darwin.json=== Install // 8a005b0c -> 2498acb1 ===
--- /tmp/old.oKV6I3 2026-07-30 17:04:20.906425020 +0000
+++ /tmp/new.xVD15p 2026-07-30 17:04:20.906425020 +0000
@@ -100,7 +100,7 @@
# first, then mount the embedded DMG and copy whichever .app it contains. This
# keeps the script version-agnostic across Homebrew bumps.
EXTRACT_DIR=$(mktemp -d /tmp/pd_extract_XXXXXX)
-unzip -q "$INSTALLER_PATH" -d "$EXTRACT_DIR"
+unzip -q "$INSTALLER_PATH" -d "$EXTRACT_DIR" || exit $?
DMG_PATH=$(find "$EXTRACT_DIR" -maxdepth 2 -name "*.dmg" | head -1)
if [ -z "$DMG_PATH" ]; then
echo "No DMG found inside the Pd archive" >&2
@@ -118,8 +118,17 @@
# copy to the applications folder
quit_and_track_application 'org.puredata.pd.pd-gui'
if [ -d "$APPDIR/$APP_NAME" ]; then
- sudo mv "$APPDIR/$APP_NAME" "$TMPDIR/$APP_NAME.bkp"
+ sudo mv "$APPDIR/$APP_NAME" "$TMPDIR/$APP_NAME.bkp" || exit $?
+fi
+if ! sudo cp -R "$APP_BUNDLE" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/$APP_NAME"
+ if [ -d "$TMPDIR/$APP_NAME.bkp" ]; then
+ sudo mv "$TMPDIR/$APP_NAME.bkp" "$APPDIR/$APP_NAME"
+ fi
+ hdiutil detach "$MOUNT_POINT" || true
+ exit 1
fi
-sudo cp -R "$APP_BUNDLE" "$APPDIR"
hdiutil detach "$MOUNT_POINT" || true
relaunch_application 'org.puredata.pd.pd-gui'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/pritunl/darwin.json=== Install // 50fa15ff -> d5a06183 ===
--- /tmp/old.bPxG7z 2026-07-30 17:04:20.964425312 +0000
+++ /tmp/new.RLWfbb 2026-07-30 17:04:20.964425312 +0000
@@ -98,5 +98,5 @@
unzip "$INSTALLER_PATH" -d "$TMPDIR"
# install pkg files
quit_and_track_application 'com.electron.pritunl'
-sudo installer -pkg "$TMPDIR/Pritunl.pkg" -target /
+sudo installer -pkg "$TMPDIR/Pritunl.pkg" -target / || exit $?
relaunch_application 'com.electron.pritunl'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/vnc-viewer/darwin.json=== Install // 932783c1 -> f79954c2 ===
--- /tmp/old.jG4Mc6 2026-07-30 17:04:21.047425731 +0000
+++ /tmp/new.bCZt35 2026-07-30 17:04:21.047425731 +0000
@@ -96,7 +96,7 @@
# install pkg files
quit_and_track_application 'com.realvnc.rvncconnect'
-sudo installer -pkg "$TMPDIR/RealVNC-Connect-Viewer-8.4.2-MacOSX-universal.pkg" -target /
+sudo installer -pkg "$TMPDIR/RealVNC-Connect-Viewer-8.4.2-MacOSX-universal.pkg" -target / || exit $?
relaunch_application 'com.realvnc.rvncconnect'
# Remove the legacy VNC Viewer (pre-rebrand bundle id com.realvnc.vncviewer) so the rebranded
# RealVNC Connect Viewer supersedes it and the patch policy converges.
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/webex/darwin.json=== Install // d105863f -> 8ff3811b ===
--- /tmp/old.IyoDuM 2026-07-30 17:04:21.100425998 +0000
+++ /tmp/new.66tgW7 2026-07-30 17:04:21.100425998 +0000
@@ -115,13 +115,24 @@
# extract contents
MOUNT_POINT=$(mktemp -d /tmp/dmg_mount_XXXXXX)
yes | hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH" || exit 1
-sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"
+if ! sudo cp -R "$MOUNT_POINT"/* "$TMPDIR"; then
+ hdiutil detach "$MOUNT_POINT" || true
+ exit 1
+fi
hdiutil detach "$MOUNT_POINT" || true
# copy to the applications folder
quit_and_track_application 'Cisco-Systems.Spark'
if [ -d "$APPDIR/Webex.app" ]; then
- sudo mv "$APPDIR/Webex.app" "$TMPDIR/Webex.app.bkp"
+ sudo mv "$APPDIR/Webex.app" "$TMPDIR/Webex.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/Webex.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/Webex.app"
+ if [ -d "$TMPDIR/Webex.app.bkp" ]; then
+ sudo mv "$TMPDIR/Webex.app.bkp" "$APPDIR/Webex.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/Webex.app" "$APPDIR"
remove_stale_upgrade_bundles
relaunch_application 'Cisco-Systems.Spark'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/wins/darwin.json=== Install // 16e109de -> 9d1f8a20 ===
--- /tmp/old.SI3fk5 2026-07-30 17:04:21.178426391 +0000
+++ /tmp/new.AYlG5q 2026-07-30 17:04:21.178426391 +0000
@@ -99,7 +99,15 @@
# copy to the applications folder
quit_and_track_application 'cools.wins.main'
if [ -d "$APPDIR/Wins.app" ]; then
- sudo mv "$APPDIR/Wins.app" "$TMPDIR/Wins.app.bkp"
+ sudo mv "$APPDIR/Wins.app" "$TMPDIR/Wins.app.bkp" || exit $?
+fi
+if ! sudo cp -R "$TMPDIR/Wins.app" "$APPDIR"; then
+ # remove the partial copy so a failed install isn't inventoried as the new
+ # version, then restore the previous version if there was one
+ sudo rm -rf "$APPDIR/Wins.app"
+ if [ -d "$TMPDIR/Wins.app.bkp" ]; then
+ sudo mv "$TMPDIR/Wins.app.bkp" "$APPDIR/Wins.app"
+ fi
+ exit 1
fi
-sudo cp -R "$TMPDIR/Wins.app" "$APPDIR"
relaunch_application 'cools.wins.main'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/worksheet-crafter/darwin.json=== Install // 545289c7 -> aa497eb5 ===
--- /tmp/old.TUIx9t 2026-07-30 17:04:21.255426779 +0000
+++ /tmp/new.VWWSjl 2026-07-30 17:04:21.257426789 +0000
@@ -96,5 +96,5 @@
# install pkg files
quit_and_track_application 'com.SchoolCraft.WillBeReplacedByQMake'
-sudo installer -pkg "$TMPDIR/worksheet-crafter_2026.2.4.pkg" -target /
+sudo installer -pkg "$TMPDIR/worksheet-crafter_2026.2.4.pkg" -target / || exit $?
relaunch_application 'com.SchoolCraft.WillBeReplacedByQMake'
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/zoom/darwin.json=== Install // ab84b572 -> 63cd9eb1 ===
--- /tmp/old.d5yONl 2026-07-30 17:04:21.313427071 +0000
+++ /tmp/new.ThbHUo 2026-07-30 17:04:21.313427071 +0000
@@ -54,7 +54,7 @@
quit_application 'us.zoom.xos' "$CONSOLE_USER"
fi
-installer -pkg "$INSTALLER_PATH" -target /
+installer -pkg "$INSTALLER_PATH" -target / || exit $?
# Restart Zoom if it was running before installation
if [[ "$ZOOM_WAS_RUNNING" == "true" ]]; then
=== Uninstall Script (no changes) === |
Member
Author
|
@allenhouchins Yes, absolutely 👍 |
jkatz01
approved these changes
Jul 30, 2026
2 tasks
1 task
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
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.
Related issue: Resolves #50056
Summary
macOS FMA install scripts never checked the exit code of the install command (
installer -pkg/cp -R) — the script's last statement is alwaysrelaunch_application, which exits 0 — so a failed install exited 0 and Fleet reported it installed.Generated scripts. The generator now propagates failure: both
installer -pkgvariants end with|| exit $?, and thecp -Rpath exits non-zero on a failed copy, removes the partial copy (so a failed fresh install isn't inventoried as the new version), and restores the app it moved aside. Regeneratedoutputs/for non-frozen generated apps are produced by theingest-maintained-appsjob, so they aren't committed here.Custom scripts. 9 of the 18 custom input scripts had the same bug and are fixed with the same pattern: Google Chrome, Zoom, Microsoft Edge, GitHub Desktop, Webex, Cycling '74 Max, Pd, Grammarly Desktop, and P4V. The DMG-based ones also now fail before removing/moving the existing app when the mount or staging copy fails, so a bad download can't leave a host with nothing. Their
outputs/*/darwin.jsonare updated in the same commit (script content + recomputed 8-char sha256 ref, versions untouched), following the precedent of #49033. Docker Desktop (set -euo pipefail), 1Password/Slack/LogiTune (installer is the last statement), and the rest already propagated errors.Frozen apps. The ingest job never rewrites frozen outputs, so the 10 frozen apps with generated scripts (adobe-acrobat-pro, comet, evernote, firealpaca, keeper-password-manager, nvidia-geforce-now, pritunl, vnc-viewer, wins, worksheet-crafter) had the fix applied directly to their published
darwin.jsonscripts — the exact text the current generator would emit, with pinned versions/URLs/hashes untouched. The 11th frozen app (logi-options+) uses a custom script that was already correct and in sync.Checklist for submitter
changes/(changes/50056-fma-install-scripts-ignore-errors).Testing
bash -n, refs matchsha256(script)[:8], refs map stays key-sorted like Go's encoder.Summary by CodeRabbit