Skip to content

fix(build): notarize macOS app bundle before stapling#1245

Merged
ErikBjare merged 1 commit intoActivityWatch:masterfrom
TimeToBuildBob:bob/fix-devrelease-notarize-app
Apr 7, 2026
Merged

fix(build): notarize macOS app bundle before stapling#1245
ErikBjare merged 1 commit intoActivityWatch:masterfrom
TimeToBuildBob:bob/fix-devrelease-notarize-app

Conversation

@TimeToBuildBob
Copy link
Copy Markdown
Contributor

Summary

  • fix the macOS notarization script to treat dist/ActivityWatch.app as a bundle directory, not a regular file
  • notarize the zipped .app bundle before stapling it, instead of skipping the app notarization step entirely
  • quote notarization/stapling paths for consistency

Problem

build-tauri.yml was failing on master in the macOS Package dmg step with:

  • Skipping: dist/ActivityWatch.app
  • The staple and validate action failed! Error 65.

The root cause was simple and dumb: scripts/notarize.sh checked the app bundle with test -f, but .app is a directory. That made the script skip app notarization entirely, then immediately try to staple an un-notarized app/dmg chain.

Validation

  • bash -n scripts/notarize.sh
  • local mocked smoke test of scripts/notarize.sh with fake xcrun/ditto, verifying the .app path now goes through notarization + stapling instead of the skip branch

Expected impact

This should unblock the failing macOS Build Tauri jobs on master, which in turn stops Create dev release from skipping due to HEAD CI failures.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 6, 2026

Greptile Summary

This PR fixes the root cause of the macOS CI failure by correcting the .app bundle check in scripts/notarize.sh from test -f (file test) to test -d (directory test), ensuring the app bundle is actually submitted for notarization before xcrun stapler is invoked. Path quoting is also improved at all call sites in both the app and DMG sections.

  • Core fix (test -ftest -d): correct and minimal — .app bundles are directories, so the old check always fell through to the skip branch, leaving the bundle un-notarized and producing Error 65 during stapling.
  • Quoting improvements: the ditto, $notarization_method, and run_stapler call sites are all quoted at the point of invocation. However, the run_stapler function body itself still has an unquoted $dist on line 46 (see inline comment).
  • Temp zip cleanup: dist/ActivityWatch.app.zip is created for the notarytool submission but never removed — a minor stale artifact left in dist/.
  • Pre-existing bug (line 58): output=xcrun altool ... is a mis-formed command that assigns the literal string \"xcrun\" to an env var and then tries to run altool as a standalone binary. The altool fallback detection has silently never worked; harmless in practice since notarytool takes priority on modern Xcode installs, but worth fixing.

Confidence Score: 4/5

Safe to merge — the core fix is correct and directly unblocks the failing macOS CI jobs; remaining issues are pre-existing or minor style concerns that do not affect the happy path.

The single test -ftest -d change is precisely the right fix for .app bundles being directories. Quoting is improved at all call sites. The only new gap introduced is the unquoted $dist inside run_stapler, which does not affect the default dist/ActivityWatch.app path (no spaces). The pre-existing altool detection bug and missing zip cleanup do not block merging.

scripts/notarize.sh — the run_stapler function body (line 46) and the altool detection line (line 58) each have minor issues worth a follow-up.

Important Files Changed

Filename Overview
scripts/notarize.sh Core test -ftest -d fix is correct and directly unblocks CI; minor unquoted $dist gap remains inside run_stapler, temp zip is not cleaned up, and a pre-existing altool detection bug is exposed

Sequence Diagram

sequenceDiagram
    participant S as notarize.sh
    participant D as ditto
    participant N as xcrun notarytool
    participant St as xcrun stapler

    S->>S: test -d dist/ActivityWatch.app
    alt .app bundle exists (directory)
        S->>D: ditto -c -k --keepParent app app.zip
        D-->>S: app.zip created
        S->>N: notarytool submit app.zip --wait
        N-->>S: notarization ticket issued
        S->>St: stapler staple app
        St-->>S: ticket stapled to bundle
        Note over S: app.zip left behind (not cleaned up)
    else .app missing
        S->>S: echo Skipping (expected .app bundle directory)
    end
    S->>S: test -f dist/ActivityWatch.dmg
    alt .dmg exists
        S->>N: notarytool submit dmg --wait
        N-->>S: notarization ticket issued
        S->>St: stapler staple dmg
        St-->>S: ticket stapled to dmg
    else .dmg missing
        S->>S: echo Skipping
    end
Loading

Comments Outside Diff (2)

  1. scripts/notarize.sh, line 46 (link)

    P2 Unquoted $dist inside run_stapler

    The PR correctly quotes every call site (e.g. run_stapler "$app" and run_stapler "$dmg"), but the actual xcrun stapler invocation inside the function still leaves $dist unquoted. A path containing spaces would be word-split here and cause the staple step to fail — inconsistent with the PR's stated goal of quoting paths for consistency.

  2. scripts/notarize.sh, line 58 (link)

    P1 Pre-existing syntax bug in altool detection

    output=xcrun altool >/dev/null 2>&1 does not run xcrun altool. In bash, a bare var=value command args prefix sets an environment variable for the command that follows. Here, output=xcrun sets the env var output to the string "xcrun", and then the shell tries to execute altool as a standalone binary — which will not be found. Since output is never read, the fix is to drop the spurious prefix entirely:

    (Note: this is a pre-existing bug, not introduced by this PR, but it means the altool fallback detection has never worked correctly.)

Reviews (1): Last reviewed commit: "fix(build): notarize macOS app bundle be..." | Re-trigger Greptile

Comment on lines +73 to +75
ditto -c -k --keepParent "$app" "$zip"
$notarization_method "$zip"
run_stapler "$app"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Temporary zip file is never cleaned up

The zip archive created by ditto at $zip (dist/ActivityWatch.app.zip) is used for notarytool submission but is never removed afterwards, leaving a stale artifact alongside the .app bundle and .dmg in dist/. Consider deleting it after stapling:

Suggested change
ditto -c -k --keepParent "$app" "$zip"
$notarization_method "$zip"
run_stapler "$app"
ditto -c -k --keepParent "$app" "$zip"
$notarization_method "$zip"
run_stapler "$app"
rm -f "$zip"

@TimeToBuildBob
Copy link
Copy Markdown
Contributor Author

LGTM. Core fix (test -ftest -d for .app bundle) is correct and directly unblocks macOS CI. Minor pre-existing issues noted by Greptile (unquoted $dist, altool fallback, zip cleanup) are non-blocking. Thanks!

@ErikBjare ErikBjare merged commit 9949fdf into ActivityWatch:master Apr 7, 2026
17 checks passed
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.

2 participants