ci(release): Merge artifacts for Craft release discovery #1249
Conversation
Craft requires a single artifact named with the commit SHA containing all release files. The current workflow uploaded separate artifacts with custom names, causing Craft to fail with 'Can't find any artifacts for revision'. Changes: - Add merge-artifacts job to build.yml that combines built-packages, spotlight-binaries, and electron-binaries into a single SHA-named artifact - Update statusProvider context to wait for 'Merge Artifacts' job - Update requireNames to match flattened file patterns
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Ui
Other
Documentation 📚Website
Other
Build / dependencies / internal 🔧CraftReleaseOther
Other
🤖 This preview updates automatically when you update the PR. |
BYK
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| - name: Download release artifacts | ||
| uses: actions/download-artifact@v5 | ||
| with: | ||
| pattern: '{built-packages,spotlight-binaries,electron-binaries}' |
There was a problem hiding this comment.
Bug: The pattern parameter for actions/download-artifact uses brace expansion ({a,b,c}), which is likely unsupported. Standard glob syntax does not include brace expansion, which is a shell feature.
Severity: CRITICAL
🔍 Detailed Analysis
The actions/download-artifact@v5 action is configured with a pattern using brace expansion: '{built-packages,spotlight-binaries,electron-binaries}'. Brace expansion is a shell feature and is not typically supported by standard glob libraries, including the one likely used by GitHub Actions (@actions/glob). The action will probably search for a single artifact with the literal name '{built-packages,spotlight-binaries,electron-binaries}', which does not exist. This will cause the artifact download to fail, breaking the subsequent merge-artifacts job and preventing the release from being created.
💡 Suggested Fix
Replace the brace expansion pattern with a method that is explicitly supported. Either call the download-artifact action multiple times, once for each artifact name, or use a wildcard pattern like * or *-binaries if appropriate, combined with merge-multiple: true to download all required artifacts into a single directory.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/build.yml#L419
Potential issue: The `actions/download-artifact@v5` action is configured with a
`pattern` using brace expansion:
`'{built-packages,spotlight-binaries,electron-binaries}'`. Brace expansion is a shell
feature and is not typically supported by standard glob libraries, including the one
likely used by GitHub Actions (`@actions/glob`). The action will probably search for a
single artifact with the literal name
`'{built-packages,spotlight-binaries,electron-binaries}'`, which does not exist. This
will cause the artifact download to fail, breaking the subsequent `merge-artifacts` job
and preventing the release from being created.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8505748
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| with: | ||
| pattern: '{built-packages,spotlight-binaries,electron-binaries}' | ||
| path: artifacts/ | ||
| merge-multiple: true |
There was a problem hiding this comment.
Artifact paths not flattened despite PR description claim
Medium Severity
The PR description claims that merge-multiple: true "flattens" artifacts, but this option only merges multiple artifacts into the same directory while preserving their internal directory structure. Files from spotlight-binaries will have paths like packages/spotlight/dist-bin/spotlight-linux-x64, not spotlight-linux-x64 at the root. The requireNames pattern /^spotlight-/ in .craft.yml expects files starting with "spotlight-" which won't match unless Craft matches against basenames rather than full relative paths. If Craft uses full paths for matching, this will cause release validation to fail.
Additional Locations (1)
| uses: actions/upload-artifact@v5 | ||
| with: | ||
| name: ${{ github.sha }} | ||
| path: artifacts/ |
There was a problem hiding this comment.
Missing error handling for empty artifact upload
Low Severity
The merged artifact upload step is missing if-no-files-found: error, unlike all other artifact uploads in this workflow (lines 128, 137, 144, 403). With the default warn behavior, if the artifacts/ directory ends up empty due to a download issue, the upload step would succeed but no artifact would be created. Craft would then fail later with a confusing "Can't find any artifacts for revision" error rather than failing at the source. Adding the explicit error check would maintain consistency and provide clearer failure messages.
Problem
Craft requires a single artifact named with the commit SHA containing all release files at the root level. The current workflow uploads separate artifacts (
built-packages,spotlight-binaries,electron-binaries) with custom names, causing Craft to fail with:Additionally, the
electron-macjob may still be running when Craft attempts to publish, causing theelectron-binariesartifact to be missing.Solution
Add a
merge-artifactsjob that:buildandelectron-macjobs to completemerge-multiple: trueChanges
merge-artifactsjob at the end of the workflowstatusProvidercontext toMerge Artifactsand updaterequireNamesto match flattened file patterns