Make macOS release resilient to stapler failures and add assistant.zip artifact#1860
Conversation
…p artifact Co-Authored-By: vargas@vellum.ai <vargas@vellum.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
Co-Authored-By: vargas@vellum.ai <vargas@vellum.ai>
| [ -f "$DMG_PATH" ] && ASSETS+=("$DMG_PATH") | ||
| [ -f "dist/vellum-assistant.zip" ] && ASSETS+=("dist/vellum-assistant.zip") | ||
| [ -f "$APPCAST" ] && ASSETS+=("$APPCAST#appcast.xml") | ||
| [ -f "build/assistant.zip" ] && ASSETS+=("build/assistant.zip#assistant.zip") |
There was a problem hiding this comment.
DevinAI this looks redundant? Looks like we have the assistant artifact on here twice
There was a problem hiding this comment.
These are actually two different files:
dist/vellum-assistant.zip— the Sparkle auto-update ZIP containing the builtVellum.app(macOS app bundle, created in the "Build ZIP for Sparkle auto-update" step)build/assistant.zip— the new zip of theassistant/source directory (your third request)
Want me to remove one, or rename build/assistant.zip to something more distinct?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3523019852
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| - name: Create GitHub Release on public repos | ||
| if: github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/') | ||
| if: always() && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')) |
There was a problem hiding this comment.
Gate release creation on successful build outcomes
Using always() here causes the release step to run after any upstream failure on main/tags, not just stapling issues. This step later deletes the existing latest release and tag before publishing new assets, so a failed build/notarization run can replace a healthy release with an incomplete one (for example, only assistant.zip), which can break normal download and update flows until another successful run.
Useful? React with 👍 / 👎.
| STAPLE_OUTPUT=$(xcrun stapler staple "$DMG_PATH" 2>&1) | ||
| STAPLE_EXIT=$? |
There was a problem hiding this comment.
Preserve stapler retries when stapling initially fails
This assignment runs outside an if guard, and GitHub Actions bash steps use -e, so a non-zero stapler exit aborts the script immediately on the first failed attempt. As a result, the retry loop and its diagnostics are skipped for transient CDN propagation failures, and because the step is continue-on-error, the workflow can proceed to release without ever completing the intended stapling/validation retries.
Useful? React with 👍 / 👎.
|
|
||
| - name: Create GitHub Release on public repos | ||
| if: github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/') | ||
| if: always() && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')) |
There was a problem hiding this comment.
🔴 always() on release step can replace a good release with a broken one after build/notarization failures
The release step at line 324 uses if: always() && (...), which causes it to run even when critical upstream steps like build, signing, or notarization fail. The step first deletes the existing latest release and tag (lines 346-347), then creates a new release from whichever files happen to exist on disk (lines 349-354).
Root Cause and Impact
If the build or notarization step fails (e.g., Xcode compilation error, notarization rejection), the DMG, Sparkle ZIP, and appcast won't exist. The intermediate steps ("Build ZIP for Sparkle", "Generate appcast.xml") use if: github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/') which implicitly includes success(), so they'll be skipped on failure. But the release step with always() still runs.
The result is:
- The existing good
latestrelease is deleted (gh release delete latest) - The existing
latesttag is deleted - A new release is created containing only
build/assistant.zip(the source zip created early in the pipeline before any build steps)
This replaces a fully functional release (with DMG, Sparkle auto-update ZIP, and appcast) with one containing only a source archive. Users downloading the latest release would get no installable artifact, and Sparkle auto-update would break.
The intent was to be resilient only to stapler failures (the step with continue-on-error: true). The condition should be scoped to allow the staple step to fail while still requiring all other steps to succeed, e.g.:
if: (success() || steps.staple.outcome == 'failure') && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/'))| if: always() && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')) | |
| if: (success() || steps.staple.outcome == 'failure') && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')) |
Was this helpful? React with 👍 or 👎 to provide feedback.
Resolved modify/delete conflict: IPCMessages.swift was moved to shared/ in this branch, while main had no modifications to port. Changes from main: - Tool result images for OpenAI/Gemini providers (#1864) - Browser screenshot in tool call results (#1863) - Qdrant vector DB clear (#1862) - Debug prompt logger truncation removal (#1861) - macOS release stapler resilience (#1860) No conflicts with shared library changes.
Captures
xcrun stapler stapleerrors (previously swallowed), makes the release step resilient to macOS deployment failures viacontinue-on-error+always(), and adds a zippedassistant/directory as a release artifact. Also bumps assistant version to 0.1.11.Summary
xcrun stapler stapleoutput is now captured into a variable and printed on every attempt, with::warning::annotations showing attempt number and exit code (previously the error was swallowed by theifconditional).continue-on-error: true, and the release step usesif: always()so it runs even if upstream steps fail. Release assets are dynamically built from whichever files exist on disk.assistant/directory and includes it as a release asset.assistant/package.json0.1.10 → 0.1.11.Review & Testing Checklist for Human
always()scope is broader than just stapler: The release step will now also run if notarization or earlier build steps fail. This could produce a release with onlyassistant.zip(no DMG) if the build itself fails. Confirm this is acceptable vs. gating on build success.(cd ../.. && zip -r clients/macos/build/assistant.zip assistant/)relative to the job'sworking-directory: clients/macos. This hasn't been tested on the actual macOS runner — consider triggering aworkflow_dispatchrun to verify.bun install, so it contains only source files (nonode_modulesor built artifacts). Confirm "fully zipped assistant directory" means repo source, not a built bundle.Notes