ci: fix appcast publish by keeping SUFeedURL=appcast.xml (mirror LockIME)#5
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
Summary by CodeRabbit
WalkthroughThe workflow now generates architecture-specific Sparkle appcast files for arm64 and x86_64, seeds each from the matching Possibly Related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-publish.yml:
- Around line 231-236: The shell step is interpolating workflow inputs directly
in the bash source, so `${{ inputs.tag }}` and `${{ inputs.channel }}` can be
turned into shell syntax before parsing. Move these values into the step’s env
for the script that builds `GA_ARGS`, then read them as quoted shell variables
inside the bash block. Update the references in the download URL prefix and the
beta-channel conditional to use the env-backed variables instead of direct
workflow expressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7425072c-3930-4380-bd5d-607764d6b8df
📒 Files selected for processing (1)
.github/workflows/build-publish.yml
…lation)
GitHub Actions expands ${{ }} before bash parses the run script, so
interpolating inputs.tag/inputs.channel (and github.repository) directly into
the body is a shell-injection surface — release.yml's version/tag are free-text
on workflow_dispatch. Pass them through the step env block and read quoted
shell variables instead. Addresses CodeRabbit review on PR #5.
…IME) The nightly/release "Build appcasts" step exited 1 right after "Wrote 1 new update ... in appcast-arm64.xml". generate_appcast derives the output appcast filename from the app's SUFeedURL basename (it can't run the app, reads Info.plist). Commit #3 set SUFeedURL to .../appcast-arm64.xml, so the tool wrote appcast-arm64.xml in every build/dist-$ARCH dir while the CI seeds, tests, and publishes the LOCAL file build/dist-$ARCH/appcast.xml -> test -f failed. Mirror LockIME instead of forcing a per-arch local name with -o: - Revert Sources/CloseUp/Info.plist SUFeedURL to .../appcast.xml. It is a runtime no-op (UpdaterDelegate.feedURLString pins an explicit per-arch URL for both arches), so its only jobs are the generate_appcast naming basis and the 0.1.0 default. With basename=appcast.xml the tool writes the local appcast.xml the CI already expects -- no -o. - The local working file in each build/dist-$ARCH is always appcast.xml (the arch is in the directory name); the per-arch feed name is applied ONLY at the gh-pages boundary (seed appcast-$ARCH.xml -> local appcast.xml; publish local appcast.xml -> appcast-$ARCH.xml). - Unlike LockIME (legacy arm64-only, so arm64's feed IS appcast.xml), CloseUp's universal 0.1.0 also runs on Intel, so the feeds stay symmetric (appcast-arm64.xml / appcast-x86_64.xml) and appcast.xml is a frozen 0.1.0 orphan the pipeline never writes -- otherwise an Intel 0.1.0 would pull an arm64 build. Also pass inputs.tag/channel + github.repository to the step via env instead of interpolating ${{ }} into the bash body (shell-injection hardening, per CodeRabbit). Verified with Sparkle's prebuilt generate_appcast: basename=appcast.xml, no -o -> writes local appcast.xml, adds the new entry, preserves a seeded old entry's signature, no stray appcast-*.xml. Docs/AGENTS.md/RUNBOOK updated; the -o approach is removed.
b164313 to
6e21e3c
Compare
Problem
The Nightly (Beta) / Release publish job fails at Build appcasts with exit 1, right after:
test -f "build/dist-$ARCH/appcast.xml"then fails.Root cause
generate_appcastcan't run the app, so it derives the output appcast filename from the app'sSUFeedURLbasename. The per-arch split (#3) setInfo.plistSUFeedURLto…/appcast-arm64.xml, so the tool writesappcast-arm64.xmlin everybuild/dist-$ARCHdir — but the CI seeds, tests, and publishes the local filebuild/dist-$ARCH/appcast.xml. Mismatch → exit 1. (The log line…in appcast-arm64.xmlis the derived name, not the file actually written.)Fix — mirror LockIME (no
-o)Sources/CloseUp/Info.plistSUFeedURL→…/appcast.xml. It is a runtime no-op (UpdaterDelegate.feedURLStringpins an explicit per-arch URL for both arches); its only jobs are thegenerate_appcastnaming basis and the 0.1.0 default. With basenameappcast.xml, the tool writes the localappcast.xmlthe CI already expects — no-oneeded.build/dist-$ARCHis alwaysappcast.xml(the arch is already in the directory name). The per-arch feed name is applied only at the gh-pages boundary: seedappcast-$ARCH.xml→ localappcast.xml; publish localappcast.xml→appcast-$ARCH.xml.appcast-arm64.xml/appcast-x86_64.xml);appcast.xmlstays a frozen 0.1.0 orphan the pipeline never writes. Unlike LockIME (legacy arm64-only, so its arm64 feed isappcast.xml), CloseUp's universal 0.1.0 also runs on Intel — arm64 entries inappcast.xmlwould push an arm64 build to an Intel 0.1.0.inputs.tag/channel+github.repositoryto the step viaenvinstead of interpolating${{ }}into the bash body (shell-injection hardening, per CodeRabbit).Verification
generate_appcast, basenameappcast.xml, no-o): writes localappcast.xml, adds the new entry, preserves a seeded old entry's signature, creates no strayappcast-*.xml.Docs updated (
AGENTS.md,docs/RUNBOOK.md); the-oapproach is removed. Rebased onto currentmain(includes #4).Note:
ci.yml(this PR's checks) builds/tests but does not callbuild-publish.yml; only the scheduled nightly / manual release dispatch exercise the publish path.