Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the tagged macOS release workflow to prevent NativePHP/Electron’s afterSign hook from notarizing during the build, and instead performs app notarization explicitly in the workflow before stapling—making artifact discovery and failure diagnostics more reliable.
Changes:
- Add a workflow-level flag to skip NativePHP’s built-in
afterSignnotarization and patch the upstream hook at runtime. - Make DMG discovery more resilient and print directory contents when expected artifacts aren’t found.
- Explicitly archive and notarize the
.appbundle in the workflow prior to stapling.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const marker = " console.log('aftersign hook triggered, start to notarize app.')\n"; | ||
| const snippet = " if (process.env.KATRA_SKIP_NATIVEPHP_AFTERSIGN_NOTARIZE === 'true') {\n console.log('skipping NativePHP afterSign notarization because the workflow notarizes artifacts explicitly.')\n return\n }\n\n"; | ||
|
|
||
| if (! pattern.test(normalized)) { | ||
| throw new Error('Unable to locate the NativePHP notarization error handler.'); | ||
| if (normalized.includes('KATRA_SKIP_NATIVEPHP_AFTERSIGN_NOTARIZE')) { | ||
| process.exit(0); | ||
| } | ||
|
|
||
| const updated = normalized.replace(pattern, "$1 throw error\n$2"); | ||
| if (! normalized.includes(marker)) { | ||
| throw new Error('Unable to locate the NativePHP afterSign hook marker.'); | ||
| } | ||
|
|
||
| const updated = normalized.replace(marker, marker + snippet); |
There was a problem hiding this comment.
The patching logic depends on an exact hard-coded marker string (including indentation and the full log message). This makes the workflow brittle to small upstream NativePHP changes (e.g., different whitespace/message), causing releases to fail even though the hook is still present. Consider matching more flexibly (e.g., a regex on the afterSign log line or function boundary) and/or validating that the inserted guard is placed inside the expected function before writing.
| echo "Discovered files under nativephp/electron/dist:" >&2 | ||
| find nativephp/electron/dist -maxdepth 3 -print | sort >&2 |
There was a problem hiding this comment.
With set -euo pipefail enabled, the debug find nativephp/electron/dist ... | sort will cause this step to exit early if nativephp/electron/dist doesn't exist (or find returns non-zero for any reason), preventing the clearer "No macOS ... was generated" errors from running. Wrap this diagnostic block so it can’t fail the step (e.g., check [[ -d nativephp/electron/dist ]] first, or append || true).
| echo "Discovered files under nativephp/electron/dist:" >&2 | |
| find nativephp/electron/dist -maxdepth 3 -print | sort >&2 | |
| if [[ -d nativephp/electron/dist ]]; then | |
| echo "Discovered files under nativephp/electron/dist:" >&2 | |
| find nativephp/electron/dist -maxdepth 3 -print | sort >&2 || true | |
| else | |
| echo "Directory nativephp/electron/dist does not exist." >&2 | |
| fi |
Closes #122.
Summary
afterSignhook from notarizing during the build jobVerification
ruby -e 'require "yaml"; YAML.load_file(".github/workflows/tagged-release.yml")'