Fix ToDesktop workflows and drop staging #1462
Conversation
dosubot
bot
added
the
size:M
📝 WalkthroughWalkthroughRemoved staging vs production build variants from GitHub Actions workflows, consolidating the build process into a single unified flow. The staging workflow is deleted, staging inputs are removed from the ToDesktop action definition and workflow callers, and the build action path is updated. Changes
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR streamlines the ToDesktop build workflows by removing the unused staging deployment path and correcting file references to use the proper composite action location.
- Deletes the unused staging workflow (
publish_staging.yml) - Corrects composite action path from
build/windows/todesktoptobuild/todesktop - Simplifies the ToDesktop composite action to use only the production build flow
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/publish_staging.yml |
Removed unused staging workflow file |
.github/workflows/todesktop.yml |
Updated composite action path and removed unused build-targets parameter |
.github/workflows/publish_all.yml |
Removed obsolete STAGING parameter |
.github/actions/build/todesktop/action.yml |
Removed STAGING input and simplified to always run production build with optional release notes updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/build/todesktop/action.yml (1)
61-89: Release notes update may fail silently if release doesn't exist or command errors.The conditional release notes logic (lines 61–89) uses
gh release viewandgh release editwithout error handling. If the release tag doesn't exist, the command fails, but the script continues without reporting failure.Add error handling around the
ghcommands:if [ -n "${{ inputs.RELEASE_TAG }}" ]; then # Create download links section DOWNLOAD_LINKS=" ### Download Latest: Mac (Apple Silicon): https://download.comfy.org/mac/dmg/arm64 Windows: https://download.comfy.org/windows/nsis/x64 <details> <summary> ### Artifacts of current release </summary> Mac (Apple Silicon): https://download.comfy.org/${BUILD_ID}/mac/dmg/arm64 Windows: https://download.comfy.org/${BUILD_ID}/windows/nsis/x64 </details>" # First get existing release notes - EXISTING_NOTES=$(gh release view ${{ inputs.RELEASE_TAG }} --json body -q .body) + EXISTING_NOTES=$(gh release view ${{ inputs.RELEASE_TAG }} --json body -q .body) || { + echo "❌ Failed to fetch release notes for tag: ${{ inputs.RELEASE_TAG }}" + exit 1 + } # Combine existing notes with download links UPDATED_NOTES="${EXISTING_NOTES}${DOWNLOAD_LINKS}" # Update the release with combined notes - gh release edit ${{ inputs.RELEASE_TAG }} --notes "$UPDATED_NOTES" + gh release edit ${{ inputs.RELEASE_TAG }} --notes "$UPDATED_NOTES" || { + echo "❌ Failed to update release notes for tag: ${{ inputs.RELEASE_TAG }}" + exit 1 + } fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/actions/build/todesktop/action.yml(2 hunks).github/workflows/publish_all.yml(0 hunks).github/workflows/publish_staging.yml(0 hunks).github/workflows/todesktop.yml(1 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/publish_all.yml
- .github/workflows/publish_staging.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (2)
.github/workflows/todesktop.yml (1)
23-28: Action path consolidation is properly implemented.The action reference has been successfully consolidated from the windows-specific path to a unified location at
./.github/actions/build/todesktop/. The old path has been removed and no stale references remain in the codebase..github/actions/build/todesktop/action.yml (1)
44-51: PUBLISH environment variable is properly consumed with no lingering STAGING logic.The
PUBLISH: truesetting is correctly passed to the build process and consumed byvite.config.tsviaprocess.env.PUBLISH. NoSTAGINGenvironment variable conditionals exist in the codebase, confirming the unconditional publish flow is as intended.
| echo "🚀 Building ToDesktop package..." | ||
| yarn run publish | tee build.log | ||
| # Extract build ID from the log | ||
| BUILD_URL=$(grep "Build complete!" build.log | head -n1 | cut -d' ' -f3) | ||
| BUILD_ID=$(echo $BUILD_URL | cut -d'/' -f6) | ||
| APP_ID=$(echo $BUILD_URL | cut -d'/' -f5) |
There was a problem hiding this comment.
Build log parsing may be fragile—consider defensive extraction.
Lines 56–58 extract BUILD_URL, BUILD_ID, and APP_ID from build.log using grep and cut. This assumes:
- The output message is exactly
"Build complete!"on the first matching line - The URL is reliably the 3rd space-separated field
- No errors occur during the
yarn run publishcommand
If ToDesktop changes output format or the command fails partially, these variables may be empty, silently corrupting download links in the release notes.
Add defensive checks after variable extraction:
BUILD_URL=$(grep "Build complete!" build.log | head -n1 | cut -d' ' -f3)
BUILD_ID=$(echo $BUILD_URL | cut -d'/' -f6)
APP_ID=$(echo $BUILD_URL | cut -d'/' -f5)
+
+ if [ -z "$BUILD_URL" ] || [ -z "$BUILD_ID" ] || [ -z "$APP_ID" ]; then
+ echo "❌ Failed to extract build information from build.log"
+ cat build.log
+ exit 1
+ fiAlternatively, consider capturing structured output from ToDesktop (if available) or improving the log parsing logic.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "🚀 Building ToDesktop package..." | |
| yarn run publish | tee build.log | |
| # Extract build ID from the log | |
| BUILD_URL=$(grep "Build complete!" build.log | head -n1 | cut -d' ' -f3) | |
| BUILD_ID=$(echo $BUILD_URL | cut -d'/' -f6) | |
| APP_ID=$(echo $BUILD_URL | cut -d'/' -f5) | |
| echo "🚀 Building ToDesktop package..." | |
| yarn run publish | tee build.log | |
| # Extract build ID from the log | |
| BUILD_URL=$(grep "Build complete!" build.log | head -n1 | cut -d' ' -f3) | |
| BUILD_ID=$(echo $BUILD_URL | cut -d'/' -f6) | |
| APP_ID=$(echo $BUILD_URL | cut -d'/' -f5) | |
| if [ -z "$BUILD_URL" ] || [ -z "$BUILD_ID" ] || [ -z "$APP_ID" ]; then | |
| echo "❌ Failed to extract build information from build.log" | |
| cat build.log | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
.github/actions/build/todesktop/action.yml lines 52-58: the script parses
BUILD_URL/BUILD_ID/APP_ID from build.log using fragile grep/cut assumptions and
doesn't check for errors; update the workflow to (1) check the exit status of
`yarn run publish` and stop with an error if it failed, (2) validate that
BUILD_URL was found (non-empty) before deriving BUILD_ID and APP_ID, and fail
the job with a clear message if not, and (3) make parsing more defensive (e.g.
use a more specific grep/regex to locate the URL or prefer structured output if
ToDesktop can emit JSON) so empty or malformed values cannot be propagated into
release notes.
Summary
Notes
This drops the old staging app flow; ToDesktop builds now use the single production path and release notes still update when
RELEASE_TAGis provided.┆Issue is synchronized with this Notion page by Unito