Fix osascript is-running checks across 15 FMA custom scripts - #49033
Conversation
…8225) osascript -e '... is running' exits 0 whether it prints true or false, so gating on the command's exit status misclassifies a fully-quit app as running. In scripts with relaunch logic (Zoom, Google Chrome) this launched the app after every patch even when the user had nothing open; in the rest it caused needless quit attempts and misleading logs. Same fix as #42951 applied to the generated helpers and #49030 applied to GitHub Desktop: capture osascript output and compare it to "true". Covers the 15 remaining custom scripts and regenerates the 13 affected darwin manifests (script-ref-only diffs, except google-chrome which also picked up the upstream 150.0.7871.115 version bump).
There was a problem hiding this comment.
Pull request overview
This PR fixes a recurring bug in Fleet-maintained app (FMA) custom macOS scripts where “is running” checks incorrectly relied on osascript’s exit status (which is 0 for both true and false). By switching to comparing osascript stdout to "true", the scripts correctly detect whether an app is actually running, preventing needless quits and unwanted relaunch behavior. It also regenerates the affected darwin.json manifests to update script refs (and includes a Chrome version bump picked up during regeneration).
Changes:
- Update 15 custom Homebrew (macOS) scripts to determine running state by comparing
osascriptoutput to"true". - Regenerate corresponding FMA
outputs/*/darwin.jsonto updateinstall_script_ref/uninstall_script_refto the corrected scripts. - Bump Google Chrome macOS manifest version and patched query to
150.0.7871.115(as part of regeneration).
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ee/maintained-apps/outputs/zoom/darwin.json | Updates Zoom install script ref to corrected running check. |
| ee/maintained-apps/outputs/slack/darwin.json | Updates Slack install script ref to corrected running check. |
| ee/maintained-apps/outputs/p4v/darwin.json | Updates P4V install/uninstall script refs to corrected running checks. |
| ee/maintained-apps/outputs/microsoft-word/darwin.json | Updates Word uninstall script ref to corrected running check. |
| ee/maintained-apps/outputs/microsoft-edge/darwin.json | Updates Edge install script ref to corrected running check. |
| ee/maintained-apps/outputs/logitune/darwin.json | Updates LogiTune install script ref to corrected running check. |
| ee/maintained-apps/outputs/grammarly-desktop/darwin.json | Updates Grammarly Desktop install script ref to corrected running check. |
| ee/maintained-apps/outputs/gpg-suite/darwin.json | Updates GPG Suite uninstall script ref to corrected running checks. |
| ee/maintained-apps/outputs/google-chrome/darwin.json | Updates Chrome install script ref and bumps version/patched query. |
| ee/maintained-apps/outputs/expressvpn/darwin.json | Updates ExpressVPN install script ref to corrected running check. |
| ee/maintained-apps/outputs/cleanmymac/darwin.json | Updates CleanMyMac uninstall script ref to corrected running check. |
| ee/maintained-apps/outputs/adobe-creative-cloud/darwin.json | Updates Adobe CC install/uninstall script refs to corrected running checks. |
| ee/maintained-apps/outputs/1password/darwin.json | Updates 1Password install script ref to corrected running check. |
| ee/maintained-apps/inputs/homebrew/scripts/zoom_install.sh | Fixes Zoom running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/slack_install.sh | Fixes Slack running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/p4v-uninstall.sh | Fixes P4V uninstall running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/p4v-install.sh | Fixes P4V install running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.sh | Fixes Edge running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/microsoft_word_uninstall.sh | Fixes Word uninstall running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/logitune-install.sh | Fixes LogiTune running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/grammarly-desktop-install.sh | Fixes Grammarly running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/gpg-suite-uninstall.sh | Fixes GPG Suite uninstall running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/google_chrome_install.sh | Fixes Chrome running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/expressvpn-install.sh | Fixes ExpressVPN running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/cleanmymac-uninstall.sh | Fixes CleanMyMac uninstall running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/adobe-cc-uninstall.sh | Fixes Adobe CC uninstall running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/adobe-cc-install.sh | Fixes Adobe CC install running detection to compare osascript output to "true". |
| ee/maintained-apps/inputs/homebrew/scripts/1password_install.sh | Fixes 1Password running detection to compare osascript output to "true". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Script Diff Resultsee/maintained-apps/outputs/1password/darwin.json=== Install // ef2a17ff -> 1d854c01 ===
--- /tmp/old.erD82q 2026-07-09 15:13:58.879759245 +0000
+++ /tmp/new.uDQSqg 2026-07-09 15:13:58.879759245 +0000
@@ -5,7 +5,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/adobe-creative-cloud/darwin.json=== Install // a331ea84 -> 2aa2127c ===
--- /tmp/old.NZVegd 2026-07-09 15:13:58.936759266 +0000
+++ /tmp/new.k4OFrK 2026-07-09 15:13:58.936759266 +0000
@@ -3,7 +3,8 @@
quit_application() {
bundle_id="$1"
timeout_duration=10
- if ! osascript -e "application id \"$bundle_id\" is running" >/dev/null 2>&1; then return; fi
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [ "$app_running" != "true" ]; then return; fi
console_user="$(stat -f "%Su" /dev/console 2>/dev/null || true)"
if [ "$(id -u)" -eq 0 ] && [ "$console_user" = "root" ]; then
echo "Skipping quit for '$bundle_id'."
=== Uninstall // e002ddef -> 05acae65 ===
--- /tmp/old.VYXccW 2026-07-09 15:13:58.951759272 +0000
+++ /tmp/new.3xNDdZ 2026-07-09 15:13:58.951759272 +0000
@@ -3,7 +3,8 @@
quit_app() {
b="$1"
# try a friendly quit if a GUI user is active
- if osascript -e "application id \"$b\" is running" >/dev/null 2>&1; then
+ app_running=$(osascript -e "application id \"$b\" is running" 2>/dev/null)
+ if [ "$app_running" = "true" ]; then
cu="$(stat -f "%Su" /dev/console 2>/dev/null || true)"
if [ "$(id -u)" -ne 0 ] || [ "$cu" != "root" ]; then
i=0ee/maintained-apps/outputs/cleanmymac/darwin.json=== Install Script (no changes) ===
=== Uninstall // 6b7419ac -> f09da8a8 ===
--- /tmp/old.i62J2g 2026-07-09 15:13:59.012759294 +0000
+++ /tmp/new.WObtpz 2026-07-09 15:13:59.012759294 +0000
@@ -11,7 +11,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fiee/maintained-apps/outputs/expressvpn/darwin.json=== Install // c5afbbfa -> efad6361 ===
--- /tmp/old.K1O1WV 2026-07-09 15:13:59.072759316 +0000
+++ /tmp/new.jhU900 2026-07-09 15:13:59.072759316 +0000
@@ -11,7 +11,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/google-chrome/darwin.json=== Install // 6981ff84 -> bf744c88 ===
--- /tmp/old.DRF1Zz 2026-07-09 15:13:59.124759335 +0000
+++ /tmp/new.ZUhXeI 2026-07-09 15:13:59.125759335 +0000
@@ -48,7 +48,8 @@
# Check if Chrome is running (only check once)
CHROME_WAS_RUNNING=false
-if osascript -e "application id \"com.google.Chrome\" is running" 2>/dev/null; then
+CHROME_RUNNING=$(osascript -e "application id \"com.google.Chrome\" is running" 2>/dev/null)
+if [[ "$CHROME_RUNNING" == "true" ]]; then
CHROME_WAS_RUNNING=true
quit_application 'com.google.Chrome' "$CONSOLE_USER"
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/gpg-suite/darwin.json=== Install Script (no changes) ===
=== Uninstall // e3b19a58 -> cf6a6135 ===
--- /tmp/old.f4kw6p 2026-07-09 15:13:59.199759362 +0000
+++ /tmp/new.G3spWM 2026-07-09 15:13:59.200759362 +0000
@@ -34,7 +34,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fiee/maintained-apps/outputs/grammarly-desktop/darwin.json=== Install // 20d5bd8f -> df3d0525 ===
--- /tmp/old.BR9Hh4 2026-07-09 15:13:59.245759379 +0000
+++ /tmp/new.M1CuF9 2026-07-09 15:13:59.246759379 +0000
@@ -11,7 +11,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/logitune/darwin.json=== Install // 584e269e -> 93aa8f2a ===
--- /tmp/old.GCSOlx 2026-07-09 15:13:59.300759399 +0000
+++ /tmp/new.93OgtB 2026-07-09 15:13:59.300759399 +0000
@@ -36,7 +36,8 @@
# Quit Logi Tune gracefully before the PKG's preinstall force-kills it. The
# PKG's default RUNAPP choice relaunches the app for logged-in console users
# after installation, so no relaunch step is needed here.
-if osascript -e "application id \"com.logitech.logitune\" is running" 2>/dev/null; then
+LOGITUNE_RUNNING=$(osascript -e "application id \"com.logitech.logitune\" is running" 2>/dev/null)
+if [[ "$LOGITUNE_RUNNING" == "true" ]]; then
quit_application 'com.logitech.logitune' "$CONSOLE_USER"
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/microsoft-edge/darwin.json=== Install // caf6f785 -> 81ff19c2 ===
--- /tmp/old.RTOLst 2026-07-09 15:13:59.359759420 +0000
+++ /tmp/new.c6wn47 2026-07-09 15:13:59.360759421 +0000
@@ -11,7 +11,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/microsoft-word/darwin.json=== Install Script (no changes) ===
=== Uninstall // 6d6819ed -> f085ec16 ===
--- /tmp/old.zZEPYu 2026-07-09 15:13:59.421759443 +0000
+++ /tmp/new.cV3LM3 2026-07-09 15:13:59.421759443 +0000
@@ -34,7 +34,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fiee/maintained-apps/outputs/p4v/darwin.json=== Install // bbe80cf5 -> 7f44eda5 ===
--- /tmp/old.ozdJyX 2026-07-09 15:13:59.478759464 +0000
+++ /tmp/new.8F2lBt 2026-07-09 15:13:59.478759464 +0000
@@ -3,7 +3,8 @@
quit_application() {
local bundle_id="$1"
local timeout_duration=10
- if ! osascript -e "application id \"$bundle_id\" is running" >/dev/null 2>&1; then return; fi
+ local app_running; app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then return; fi
local console_user; console_user=$(stat -f "%Su" /dev/console)
if [[ $EUID -eq 0 && "$console_user" == "root" ]]; then echo "Skipping quit for '$bundle_id'."; return; fi
echo "Quitting '$bundle_id'..."
=== Uninstall // 428bb3f6 -> f67991ce ===
--- /tmp/old.2ZfQKO 2026-07-09 15:13:59.494759469 +0000
+++ /tmp/new.q2etpl 2026-07-09 15:13:59.494759469 +0000
@@ -3,7 +3,8 @@
quit_application() {
local bundle_id="$1"
local timeout_duration=10
- if ! osascript -e "application id \"$bundle_id\" is running" >/dev/null 2>&1; then return; fi
+ local app_running; app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then return; fi
local console_user; console_user=$(stat -f "%Su" /dev/console)
if [[ $EUID -eq 0 && "$console_user" == "root" ]]; then return; fi
SECONDS=0ee/maintained-apps/outputs/slack/darwin.json=== Install // 6025885d -> 92608bef ===
--- /tmp/old.qAbcPj 2026-07-09 15:13:59.553759491 +0000
+++ /tmp/new.JBWD3h 2026-07-09 15:13:59.553759491 +0000
@@ -5,7 +5,9 @@
local timeout_duration=10
# check if the application is running
- if ! osascript -e "application id \"$bundle_id\" is running" 2>/dev/null; then
+ local app_running
+ app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null)
+ if [[ "$app_running" != "true" ]]; then
return
fi
=== Uninstall Script (no changes) ===ee/maintained-apps/outputs/zoom/darwin.json=== Install // 05e6a85c -> ab84b572 ===
--- /tmp/old.PBgieh 2026-07-09 15:13:59.605759510 +0000
+++ /tmp/new.mhAjTb 2026-07-09 15:13:59.605759510 +0000
@@ -48,7 +48,8 @@
# Check if Zoom is running
ZOOM_WAS_RUNNING=false
-if osascript -e "application id \"us.zoom.xos\" is running" 2>/dev/null; then
+ZOOM_RUNNING=$(osascript -e "application id \"us.zoom.xos\" is running" 2>/dev/null)
+if [[ "$ZOOM_RUNNING" == "true" ]]; then
ZOOM_WAS_RUNNING=true
quit_application 'us.zoom.xos' "$CONSOLE_USER"
fi
=== Uninstall Script (no changes) === |
WalkthroughThis PR refactors macOS install/uninstall Bash scripts for numerous Fleet-maintained apps (1Password, Adobe Creative Cloud, CleanMyMac, ExpressVPN, Google Chrome, GPG Suite, Grammarly Desktop, Logi Tune, Microsoft Edge, Microsoft Word, P4V, Slack, Zoom). The primary change replaces inline osascript exit-status checks for "is app running" with explicit variable assignment (e.g., app_running, CHROME_RUNNING, ZOOM_RUNNING) compared against the string "true". Adobe Creative Cloud also gets updated quit/uninstall/install flow logic, and P4V removes some status echo output. Corresponding darwin.json output manifests are updated with new script references and embedded content, including a Chrome version bump and Slack console-user guard expansion. 🚥 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 |
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 `@ee/maintained-apps/outputs/slack/darwin.json`:
- Line 20: The Slack uninstall guard in quit_application was widened in the
generated darwin.json script and should match the source behavior unless that
broader check is intentional. Update the console-user validation around
console_user to restore the original root-only gating used by the uninstall
script, keeping the existing quit_application flow and the Slack bundle ID
cleanup calls unchanged.
🪄 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: a62c3eea-1c41-4e4b-b2b1-f09e643f4d18
📒 Files selected for processing (28)
ee/maintained-apps/inputs/homebrew/scripts/1password_install.shee/maintained-apps/inputs/homebrew/scripts/adobe-cc-install.shee/maintained-apps/inputs/homebrew/scripts/adobe-cc-uninstall.shee/maintained-apps/inputs/homebrew/scripts/cleanmymac-uninstall.shee/maintained-apps/inputs/homebrew/scripts/expressvpn-install.shee/maintained-apps/inputs/homebrew/scripts/google_chrome_install.shee/maintained-apps/inputs/homebrew/scripts/gpg-suite-uninstall.shee/maintained-apps/inputs/homebrew/scripts/grammarly-desktop-install.shee/maintained-apps/inputs/homebrew/scripts/logitune-install.shee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.shee/maintained-apps/inputs/homebrew/scripts/microsoft_word_uninstall.shee/maintained-apps/inputs/homebrew/scripts/p4v-install.shee/maintained-apps/inputs/homebrew/scripts/p4v-uninstall.shee/maintained-apps/inputs/homebrew/scripts/slack_install.shee/maintained-apps/inputs/homebrew/scripts/zoom_install.shee/maintained-apps/outputs/1password/darwin.jsonee/maintained-apps/outputs/adobe-creative-cloud/darwin.jsonee/maintained-apps/outputs/cleanmymac/darwin.jsonee/maintained-apps/outputs/expressvpn/darwin.jsonee/maintained-apps/outputs/google-chrome/darwin.jsonee/maintained-apps/outputs/gpg-suite/darwin.jsonee/maintained-apps/outputs/grammarly-desktop/darwin.jsonee/maintained-apps/outputs/logitune/darwin.jsonee/maintained-apps/outputs/microsoft-edge/darwin.jsonee/maintained-apps/outputs/microsoft-word/darwin.jsonee/maintained-apps/outputs/p4v/darwin.jsonee/maintained-apps/outputs/slack/darwin.jsonee/maintained-apps/outputs/zoom/darwin.json
| "refs": { | ||
| "6025885d": "#!/bin/bash\n\nquit_application() {\n local bundle_id=\"$1\"\n local timeout_duration=10\n\n # check if the application is running\n if ! osascript -e \"application id \\\"$bundle_id\\\" is running\" 2>/dev/null; then\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ $EUID -eq 0 && \"$console_user\" == \"root\" ]]; then\n echo \"Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'.\"\n return\n fi\n\n echo \"Quitting application '$bundle_id'...\"\n\n # try to quit the application within the timeout period\n local quit_success=false\n SECONDS=0\n while (( SECONDS < timeout_duration )); do\n if osascript -e \"tell application id \\\"$bundle_id\\\" to quit\" >/dev/null 2>&1; then\n if ! pgrep -f \"$bundle_id\" >/dev/null 2>&1; then\n echo \"Application '$bundle_id' quit successfully.\"\n quit_success=true\n break\n fi\n fi\n sleep 1\n done\n\n if [[ \"$quit_success\" = false ]]; then\n echo \"Application '$bundle_id' did not quit.\"\n fi\n}\n\nquit_application 'com.tinyspeck.slackmacgap'\ninstaller -pkg \"$INSTALLER_PATH\" -target /\n\n", | ||
| "92608bef": "#!/bin/bash\n\nquit_application() {\n local bundle_id=\"$1\"\n local timeout_duration=10\n\n # check if the application is running\n local app_running\n app_running=$(osascript -e \"application id \\\"$bundle_id\\\" is running\" 2>/dev/null)\n if [[ \"$app_running\" != \"true\" ]]; then\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ $EUID -eq 0 && \"$console_user\" == \"root\" ]]; then\n echo \"Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'.\"\n return\n fi\n\n echo \"Quitting application '$bundle_id'...\"\n\n # try to quit the application within the timeout period\n local quit_success=false\n SECONDS=0\n while (( SECONDS < timeout_duration )); do\n if osascript -e \"tell application id \\\"$bundle_id\\\" to quit\" >/dev/null 2>&1; then\n if ! pgrep -f \"$bundle_id\" >/dev/null 2>&1; then\n echo \"Application '$bundle_id' quit successfully.\"\n quit_success=true\n break\n fi\n fi\n sleep 1\n done\n\n if [[ \"$quit_success\" = false ]]; then\n echo \"Application '$bundle_id' did not quit.\"\n fi\n}\n\nquit_application 'com.tinyspeck.slackmacgap'\ninstaller -pkg \"$INSTALLER_PATH\" -target /\n\n", | ||
| "d3fcc8c2": "#!/bin/bash\n\n# variables\nAPPDIR=\"/Applications/\"\nLOGGED_IN_USER=$(scutil <<< \"show State:/Users/ConsoleUser\" | awk '/Name :/ { print $3 }')\n# functions\n\nquit_application() {\n local bundle_id=\"$1\"\n local timeout_duration=10\n\n # check if the application is running\n local app_running\n app_running=$(osascript -e \"application id \\\"$bundle_id\\\" is running\" 2>/dev/null)\n if [[ \"$app_running\" != \"true\" ]]; then\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ -z \"$console_user\" || \"$console_user\" == \"root\" || \"$console_user\" == \"loginwindow\" ]]; then\n echo \"Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'.\"\n return\n fi\n\n echo \"Quitting application '$bundle_id'...\"\n\n # try to quit the application within the timeout period\n local quit_success=false\n SECONDS=0\n while (( SECONDS < timeout_duration )); do\n if osascript -e \"tell application id \\\"$bundle_id\\\" to quit\" >/dev/null 2>&1; then\n if ! pgrep -f \"$bundle_id\" >/dev/null 2>&1; then\n echo \"Application '$bundle_id' quit successfully.\"\n quit_success=true\n break\n fi\n fi\n sleep 1\n done\n\n if [[ \"$quit_success\" = false ]]; then\n echo \"Application '$bundle_id' did not quit.\"\n fi\n}\n\n\ntrash() {\n local logged_in_user=\"$1\"\n local target_file=\"$2\"\n local timestamp=\"$(date +%Y-%m-%d-%s)\"\n local rand=\"$(jot -r 1 0 99999)\"\n\n # replace ~ with /Users/$logged_in_user\n if [[ \"$target_file\" == ~* ]]; then\n target_file=\"/Users/$logged_in_user${target_file:1}\"\n fi\n\n local trash=\"/Users/$logged_in_user/.Trash\"\n\n # If the target contains glob characters, expand it and move each match.\n if [[ \"$target_file\" == *[*?[]* ]]; then\n local file file_name\n local matched=false\n local i=0\n # compgen -G expands the (quoted) pattern itself, so paths containing\n # spaces glob correctly; reading line by line keeps each match intact.\n while IFS= read -r file; do\n [[ -n \"$file\" ]] || continue\n [[ -e \"$file\" || -L \"$file\" ]] || continue\n matched=true\n i=$((i + 1))\n file_name=\"$(basename \"$file\")\"\n echo \"removing $file.\"\n # The per-match counter keeps matches that share a basename from\n # overwriting each other in the trash.\n mv -f \"$file\" \"$trash/${file_name}_${timestamp}_${rand}_${i}\"\n done < <(compgen -G \"$target_file\" 2>/dev/null)\n if [[ \"$matched\" == false ]]; then\n echo \"$target_file doesn't exist.\"\n fi\n return\n fi\n\n local file_name=\"$(basename \"${target_file}\")\"\n\n if [[ -e \"$target_file\" ]]; then\n echo \"removing $target_file.\"\n mv -f \"$target_file\" \"$trash/${file_name}_${timestamp}_${rand}\"\n else\n echo \"$target_file doesn't exist.\"\n fi\n}\n\nquit_application 'com.tinyspeck.slackmacgap'\nsudo rm -rf \"$APPDIR/Slack.app\"\ntrash $LOGGED_IN_USER '/Library/Logs/DiagnosticReports/Slack_*'\ntrash $LOGGED_IN_USER '~/Library/Application Scripts/com.tinyspeck.slackmacgap'\ntrash $LOGGED_IN_USER '~/Library/Application Support/com.apple.sharedfilelist/com.apple.LSSharedFileList.ApplicationRecentDocuments/com.tinyspeck.slackmacgap.sfl*'\ntrash $LOGGED_IN_USER '~/Library/Application Support/Slack'\ntrash $LOGGED_IN_USER '~/Library/Caches/com.tinyspeck.slackmacgap*'\ntrash $LOGGED_IN_USER '~/Library/Containers/com.tinyspeck.slackmacgap*'\ntrash $LOGGED_IN_USER '~/Library/Cookies/com.tinyspeck.slackmacgap.binarycookies'\ntrash $LOGGED_IN_USER '~/Library/Group Containers/*.com.tinyspeck.slackmacgap'\ntrash $LOGGED_IN_USER '~/Library/Group Containers/*.slack'\ntrash $LOGGED_IN_USER '~/Library/HTTPStorages/com.tinyspeck.slackmacgap*'\ntrash $LOGGED_IN_USER '~/Library/Logs/Slack'\ntrash $LOGGED_IN_USER '~/Library/Preferences/ByHost/com.tinyspeck.slackmacgap.ShipIt.*.plist'\ntrash $LOGGED_IN_USER '~/Library/Preferences/com.tinyspeck.slackmacgap*'\ntrash $LOGGED_IN_USER '~/Library/Saved Application State/com.tinyspeck.slackmacgap.savedState'\ntrash $LOGGED_IN_USER '~/Library/WebKit/com.tinyspeck.slackmacgap'\n" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if the Slack uninstall source script has the expanded console_user guard.
# Look for the source uninstall script and compare its quit_application guard.
# Find the Slack uninstall source script
fd -t f 'slack.*uninstall' ee/maintained-apps/inputs/
# Also search for the loginwindow check across all scripts to see which scripts use it
rg -n 'loginwindow' ee/maintained-apps/inputs/homebrew/scripts/ --glob '*.sh'Repository: fleetdm/fleet
Length of output: 2246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate every Slack uninstall/install script and the generated output entry.
git ls-files | rg -n 'slack|Slack|tinyspeck\.slackmacgap|com\.tinyspeck\.slackmacgap'
printf '\n-- quit_application / console_user guards in source scripts --\n'
rg -n 'quit_application|console_user|loginwindow|EUID -eq 0' ee/maintained-apps/inputs ee/maintained-apps/outputs --glob '*.sh' --glob '*.ps1' --glob '*.json'
printf '\n-- Slack uninstall source candidates --\n'
fd -t f -i 'slack' ee/maintained-apps/inputs ee/maintained-apps/outputsRepository: fleetdm/fleet
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '-- files with slack_uninstall --'
git ls-files | rg 'slack_uninstall|Slack_uninstall|slack-uninstall|uninstall.*slack|slack.*uninstall'
printf '\n%s\n' '-- source/golden uninstall script snippets --'
for f in \
server/mdm/maintainedapps/testdata/scripts/slack_uninstall.golden.sh \
ee/maintained-apps/inputs/winget/scripts/slack_uninstall.ps1 \
ee/maintained-apps/inputs/homebrew/scripts/slack_install.sh \
ee/maintained-apps/outputs/slack/darwin.json
do
if [[ -f "$f" ]]; then
echo "## $f"
rg -n -C 3 'console_user|loginwindow|EUID -eq 0|quit_application|quit_and_track_application' "$f"
echo
fi
doneRepository: fleetdm/fleet
Length of output: 1321
Restore the Slack uninstall guard
ee/maintained-apps/outputs/slack/darwin.json now skips empty/loginwindow console users, but the source uninstall script still uses [[ $EUID -eq 0 && "$console_user" == "root" ]]. Revert this change unless the wider guard is intentional.
🤖 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/outputs/slack/darwin.json` at line 20, The Slack uninstall
guard in quit_application was widened in the generated darwin.json script and
should match the source behavior unless that broader check is intentional.
Update the console-user validation around console_user to restore the original
root-only gating used by the uninstall script, keeping the existing
quit_application flow and the Slack bundle ID cleanup calls unchanged.
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **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 always `relaunch_application`, which exits 0 — so a failed install exited 0 and Fleet reported it installed. **Generated scripts.** The generator now propagates failure: both `installer -pkg` variants end with `|| exit $?`, and the `cp -R` path 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. Regenerated `outputs/` for non-frozen generated apps are produced by the `ingest-maintained-apps` job, 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.json` are 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.json` scripts — 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 - [x] Changes file added for user-visible changes in `changes/` (`changes/50056-fma-install-scripts-ignore-errors`). - [x] Untrusted data interpolated into shell scripts is validated against shell metacharacters. (No new untrusted interpolation: the guard reuses the same curated cask-derived name the adjacent lines already interpolate.) ## Testing - [x] Added/updated automated tests (three generator tests: pkg, pkg-with-choices, cp-R restore — the last now pins the exact emitted block). - [x] All 19 updated output manifests validated: embedded scripts pass `bash -n`, refs match `sha256(script)[:8]`, refs map stays key-sorted like Go's encoder. - [x] QA'd all new/changed functionality manually. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * macOS Fleet-maintained app installations now fail fast when installers, DMG extraction/mounting, or app copy steps error. * If an upgrade fails, the system removes any partial app and restores the previously installed version when available. * Improved robustness during app staging/copying, including safer handling of paths with spaces or special characters. * **Tests** * Added unit coverage to verify installer failure propagation and rollback behavior. * **Documentation** * Clarified that the install-script error handling applies to both generated and custom scripts, including already-published frozen apps. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Allen Houchins <allenhouchins@mac.com>
Related issue: Resolves #48638, resolves #48225
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
Details
Follow-up to #49030 (GitHub Desktop, #48639), which fixed one instance of this bug. An audit for the raw pattern found 15 more custom FMA scripts carrying the same broken check that #42951 fixed in the generated helpers: gating on the exit status of
osascript -e 'application id "..." is running'. osascript exits 0 whether it printstrueorfalse, so a fully-quit app is misclassified as running whenever the bundle id resolves.Impact by script:
zoom_install.sh(Zoom Fleet-maintained app relaunches Zoom after a patch even when it had no open window (macOS) #48638) andgoogle_chrome_install.sh(Google Chrome Fleet-maintained app relaunches Chrome after a patch/update even when it had no open window (macOS) #48225) set*_WAS_RUNNING=trueunconditionally and reopen the app afterinstaller, even when the user had nothing open.tell application id X to quitagainst a not-running app can briefly launch it to deliver the quit event, so these aren't purely cosmetic either.The fix is the same one-line pattern everywhere, style-matched to each script (
local/POSIX[ ]/top-level variants preserved): capture osascript output and compare it to"true". No other behavior changed — this PR deliberately does not touch relaunch methods or console-user guards.Regenerated the 13 affected darwin manifests with
go run ./cmd/maintained-apps -slug <slug>. All diffs are script-ref-only exceptgoogle-chrome/darwin.json, which also picked up the legitimate upstream 150.0.7871.115 version bump during regeneration (the daily ingest cron would publish it tonight regardless). An unrelatedgoogle-chrome/windows.jsonwinget bump was excluded.Manual QA
Reproduced the bug live on macOS with the shipped Zoom script logic (ref
05e6a85c) against a fully-quit Zoom (verifiedis running=false, zero processes): the exit-status check setZOOM_WAS_RUNNING=trueand the relaunch step launched Zoom — exactly the customer report, no background helpers needed. The corrected output-compare check on the same state correctly reported not running. Equivalent verification for the shared-helper variant was done against GitHub Desktop in #49030 (both the fully-quit and running→quit→relaunch paths).Verified for all 16 scripts:
bash -npasses, noif [!] osascript -e "application id ...pattern remains anywhere underinputs/homebrew/scripts/, and every regenerated manifest ref carries the output-compare check.Remaining by-design behavior (unchanged): an app running with a dock icon but no visible window is genuinely running and will still be quit and relaunched; window-aware relaunching would be a separate enhancement.
Summary by CodeRabbit