Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions .github/workflows/build-and-release-macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ jobs:
echo "build_version=$BUILD_VERSION" >> "$GITHUB_OUTPUT"
echo "Display version: $DISPLAY_VERSION / Build version: $BUILD_VERSION"

- name: Zip assistant directory
run: |
mkdir -p build
(cd ../.. && zip -r clients/macos/build/assistant.zip assistant/)
ls -lh build/assistant.zip

- name: Install Bun
uses: oven-sh/setup-bun@v2
with:
Expand Down Expand Up @@ -215,20 +221,27 @@ jobs:
echo "Notarization succeeded"

- name: Staple notarization ticket
id: staple
continue-on-error: true
run: |
# Apple's CDN can take several minutes to propagate the ticket after notarization
MAX_ATTEMPTS=15
WAIT_SECS=60
for i in $(seq 1 $MAX_ATTEMPTS); do
if xcrun stapler staple "$DMG_PATH" 2>&1; then
STAPLE_OUTPUT=$(xcrun stapler staple "$DMG_PATH" 2>&1)
STAPLE_EXIT=$?
Comment on lines +231 to +232
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

if [ $STAPLE_EXIT -eq 0 ]; then
echo "Stapling succeeded on attempt $i"
echo "$STAPLE_OUTPUT"
break
fi
echo "::warning::Stapling attempt $i/$MAX_ATTEMPTS failed (exit code $STAPLE_EXIT):"
echo "$STAPLE_OUTPUT"
if [ "$i" -eq "$MAX_ATTEMPTS" ]; then
echo "::error::Stapling failed after $MAX_ATTEMPTS attempts"
exit 1
fi
echo "Stapling attempt $i failed, waiting ${WAIT_SECS}s for CDN propagation..."
echo "Waiting ${WAIT_SECS}s for CDN propagation..."
sleep $WAIT_SECS
# Increase wait time for subsequent attempts (60, 75, 90, 105, ...)
WAIT_SECS=$((WAIT_SECS + 15))
Expand Down Expand Up @@ -308,7 +321,7 @@ jobs:
echo "Appcast generated for $REPO"

- 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/'))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🔴 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:

  1. The existing good latest release is deleted (gh release delete latest)
  2. The existing latest tag is deleted
  3. 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/'))
Suggested change
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/'))
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
run: |
Expand All @@ -333,11 +346,17 @@ jobs:
gh release delete latest --repo "$REPO" --yes 2>/dev/null || true
gh api "repos/$REPO/git/refs/tags/latest" -X DELETE 2>/dev/null || true

# Create new release with DMG + ZIP + appcast
# Build asset list from available files
ASSETS=()
[ -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")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DevinAI this looks redundant? Looks like we have the assistant artifact on here twice

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are actually two different files:

  • dist/vellum-assistant.zip — the Sparkle auto-update ZIP containing the built Vellum.app (macOS app bundle, created in the "Build ZIP for Sparkle auto-update" step)
  • build/assistant.zip — the new zip of the assistant/ source directory (your third request)

Want me to remove one, or rename build/assistant.zip to something more distinct?


echo "Release assets: ${ASSETS[*]}"

gh release create latest \
"$DMG_PATH" \
dist/vellum-assistant.zip \
"$APPCAST#appcast.xml" \
"${ASSETS[@]}" \
--repo "$REPO" \
--title "Vellum $VERSION" \
--notes "$RELEASE_NOTES" \
Expand Down
2 changes: 1 addition & 1 deletion assistant/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "vellum-assistant",
"version": "0.1.10",
"version": "0.1.11",
"type": "module",
"bin": {
"vellum": "./src/index.ts"
Expand Down