release: draft until the artifacts exist; derive ci-success from needs (#323) - #329
Conversation
`ci-success` listed its members twice — in `needs:` and again in a
hand-written `results` string — and the two drifted twice. Membership is
now derived from `toJSON(needs)`, which ends the class rather than adding
a third thing to keep in sync. The JSON arrives through `env:` and is
never interpolated into the shell body: `${{ }}` inside `run:` is textual
substitution, i.e. a script-injection surface.
Behavior is preserved and slightly hardened. The allowlist (success or
skipped; anything else fails) stays — a denylist once let `abandoned`
through and greenwashed #306. Added: an explicit assertion that `changes`
itself succeeded, because if it fails every other job reports `skipped`
and the allowlist would accept the lot; and a guard that the needs list
is non-empty, since `release.yml`'s ci-gate trusts this check. All eight
cases were exercised against the jq locally, including the #306
`abandoned` shape.
## Two lanes promoted, one deliberately not
The plan required re-establishing reliability before promoting anything,
because the old "21 success / 0 failures" evidence was measuring nothing:
`continue-on-error` makes the job conclusion green by construction.
The obvious fix — query step conclusions instead — is also worthless. The
REST jobs API exposes `conclusion` and not `outcome`, so a
`continue-on-error` step that exited 1 is reported as `success` there
too. Every measurement below therefore comes from the step's LOG BODY,
across 21 `main` runs from 2026-07-31 to 2026-08-09.
* `e2e-gtk-wayland` — 21/21 clean. `continue-on-error` removed, added to
`ci-success`.
* `e2e-gtk`'s real-input step — 21/21 clean. `continue-on-error` removed.
The plan had dispositioned this one out of scope on the grounds that
`e2e-gtk` "is not in ci-success"; it is, so a soft step inside it made
the required gate only a partial one.
* `e2e-gtk-wayland-drag` — **3 of the same 21 runs FAILED**, all with
`FAIL (real-input required): window_metrics returned no window size`.
It keeps `continue-on-error` and stays out of `ci-success`. Promoting
it is now a separate, tracked decision (#328) rather than a pending
one, and the reason is written where the next person will read it.
That leaves exactly one `continue-on-error` in the file, on the one lane
whose history does not support promotion — an invariant worth keeping.
Refs #323.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
The Release was created public and empty, before a single artifact was
built. Three things went wrong with that, and the first is the one that
actually ships a wrong package:
1. `dispatch-apt-charliek` fired on `needs: linux` — before the DMG, and
before anything guaranteed the release was complete. apt-charliek's
`collect-debs.sh` walks releases newest-first and only *warns* past one
that carries no matching .deb, so it silently republishes the PREVIOUS
version. (The precise mechanism matters: apt-charliek authenticates
with a repo-scoped `GITHUB_TOKEN` that has no push access to
`charliek/roost`, and GitHub lists drafts only to callers with push
access — so a draft is invisible to it rather than merely sorted last.)
2. A failed `linux` or `mac` left a public, empty release behind.
3. `create-release` blindly reused any existing release, so a re-run after
a publish would `--clobber` new assets straight into a live one.
The job graph is now:
create-release (--draft)
-> linux, mac
-> publish-release (asserts the artifact set, then flips)
-> appcast, dispatch-apt-charliek
`publish-release` is the one irreversible step, so the assertions live
there, where the fix is still "delete the draft and re-run": still a
draft, and an amd64 .deb, an arm64 .deb and a DMG each present exactly
once, non-empty, and named for the tag. It is idempotent — an
already-published release is a notice and exit 0, not a failure. Plain
`needs:` with no `if: always()` is what makes a failed build leave the
draft unpublished.
`create-release` now fails closed: reusing an existing DRAFT is the
re-run case and still fine, but reusing a PUBLISHED release exits 1.
## The appcast had to move, not just be reordered
`update-appcast.py` builds `https://github.com/{repo}/releases/download/
{tag}/{dmg}` by string construction and pushes it to Pages. Draft asset
URLs are not publicly resolvable, so leaving the appcast in `mac` would
have published a feed pointing at a 404 — and Sparkle has no fallback for
a dead enclosure.
Signing stays in `mac`, which is the only job holding the Sparkle SPM
artifacts that carry `sign_update`. The whole `sign.txt` travels forward
as a build artifact, because it carries both `sparkle:edSignature` and
`length` and the script needs both. The new `appcast` job range-GETs the
enclosure URL and refuses to commit if it does not resolve.
`update-appcast.py` needed no changes — verified by reading it and by
running it in a scratch directory with no `mac/build/` present: it never
opens the DMG, taking `length` from `sign.txt`.
Verified empirically before relying on it (plan D2.6): `gh release
upload --clobber` works against a draft, repeatedly, and creating a draft
does not create or move a git tag. Probed with a throwaway draft on this
repo, then deleted; `git ls-remote --tags` confirmed clean.
Two deliberate deviations from the plan:
* `appcast` gets `permissions: contents: read`, not `write`. Nothing in
it writes via `GITHUB_TOKEN` — the commit is pushed with the
release-bot App token, which is the only identity in main's ruleset
bypass list (#136). `write` would be inert privilege.
* `ROOST_REPO` is now passed to `update-appcast.py` explicitly, so the
URL the job verifies is byte-identical to the URL the script writes.
Preserved unchanged: prerelease detection, CHANGELOG extraction and the
`--generate-notes` fallback, the Gatekeeper first-launch note, the
notarization gating, the apt dispatch's two skip paths, and the
concurrency group.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
The pipeline now has an irreversible step in the middle of it, and a class of half-finished states that did not exist before: a draft holding some assets, a published release with a stale appcast, a whole-workflow re-run that is refused on purpose. None of that is discoverable from the workflow file at 3am. Adds a "Draft-until-complete, and how to recover" section covering each state — build job failed, assertions failed, re-running after a successful release, and the appcast failing *after* the point of no return — plus four new rows in the symptom table and the corrected job list. The appcast failure is stated plainly for what it is: the release is live and correct, only in-app updates are not offered yet, and re-running one job fixes it. That is the trade the job split buys, and it is the right way round — a feed published ahead of the release points every macOS updater at a 404. Also refreshed the stale references to the appcast living inside the `mac` job, and added `ROOST_REPO` to the documented environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
📝 WalkthroughWalkthroughThe pull request makes GTK and Wayland checks required CI gates. It changes releases to remain drafts until platform artifacts pass validation, then publishes the release before running Sparkle appcast updates and APT dispatch. ChangesCI gate enforcement
Release publication pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LinuxBuild
participant MacBuild
participant PublishRelease
participant GitHubRelease
participant Appcast
LinuxBuild->>GitHubRelease: Upload Linux packages
MacBuild->>GitHubRelease: Upload signed DMG
PublishRelease->>GitHubRelease: Validate draft and expected assets
PublishRelease->>GitHubRelease: Publish release
Appcast->>GitHubRelease: Verify public DMG URL
Appcast->>GitHubRelease: Update signed appcast
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Note for the record: CodeRabbit did not actually review this PR — it hit its Fair Usage rate limit and reported |
CodeRabbit was rate-limited on this PR and reported `pass` without reviewing it, so codex reviewed the release path in its place. It found four things, and the first is the one that mattered. **The draft check failed OPEN.** `if [ "$(gh release view … isDraft)" != "true" ]` puts the command substitution inside a test, where `set -e` does not fire. A transient API error yields an empty string, empty is not "true", and the job then reported "already published", exited 0, and never published — while `dispatch-apt-charliek` ran against a still-draft release. That is precisely the bug this job exists to prevent, reachable by a single flaky API call. The flag is now read into a variable (a standalone assignment DOES propagate the failure), `false` means already-published, `true` means publish, and anything else fails closed. **The asset assertions accepted a truncated or stale set.** They proved each expected name existed exactly once and was non-zero, which passes for three 1-byte files, and said nothing about extra assets — so a reused draft still carrying `roost_<older-version>_amd64.deb` would publish, and apt-charliek globs `roost_*.deb`. Now: a 1 MiB floor rather than "non-empty" (every artifact here is multi-megabyte, so `> 0` waves through exactly the corruption worth catching), and any unexpected asset is refused. Exercised standalone against ten inputs, including the exact one codex constructed to slip through: happy path, all-size-1-plus-stale-DMG, stale deb from a previous version, truncated DMG, zero-byte deb, missing arm64, no assets, duplicate name, and both prerelease version shapes (`v0.0.18-rc1`, `v1.2.3-rc-2` — only the first hyphen becomes a tilde). **A mac re-run collided with its own artifact.** Artifacts are immutable, so re-running just the mac job after a later step failed died on the existing `sparkle-sign` name instead of recovering. `overwrite: true`. **Four RELEASING.md statements were wrong**, one of them pre-existing and load-bearing: prereleases do NOT skip Sparkle signing. The signing step requires the secret unconditionally; only the separate throwaway-*key* guard is prerelease-exempt, so "cut it as a beta" was advice that would have failed the same way. Also corrected: apt dispatch and appcast are parallel siblings so apt may not have fired; a failing DMG-URL check means the release is NOT fine; and the assertion now does catch differently-named extras. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
shellcheck SC2086 on the deliberate word-split. Reworked so nothing is unquoted rather than silencing it: the expected list becomes newline- separated via `tr`, and the error report pipes through `sed` instead of splitting on printf's argument list. Same ten-case unit test still behaves identically. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
|
Codex reviewed this in CodeRabbit's place (it was rate-limited) and found four issues, all now fixed in
Re-requesting CodeRabbit now that its cooldown has passed. |
|
@coderabbitai full review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
The hosted CodeRabbit was still rate-limited, so this pass came from the `coderabbit` CLI against the branch diff. Two findings, both real. * **`ci-success` only rejected fewer than two upstream jobs.** Deriving membership from `needs` means a job DELETED from that list silently stops being gated — the gate still passes, just over less. The floor is the current 12 now. It is one integer rather than a second copy of the list, so it cannot drift the way the hand-written `results` string did: adding a gate is free, and lowering the number has to be deliberate. * **The DMG-recovery table row contradicted the prose two sections above it.** It said "the release itself is fine" while the narrative correctly says macOS users have nothing to download in that state, and it implied re-running `appcast` alone would fix a missing asset. It cannot; the DMG has to be re-uploaded first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
708-713: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope the release-bot token; the actionlint hit here is a false positive.
Two separate points.
zizmor is right. This token inherits every permission the App installation holds, while the sibling mint at
.github/workflows/release.yml:848-853scopes itself withowner+repositories. The token here only needs to pushdocs/appcast.xmlto this repository. Downscope it.actionlint's
client-id/ missingapp-iderrors at lines 710 and 712 are stale action metadata.client-idis a first-class input ofactions/create-github-app-tokensince v3.1.0, andapp-idis deprecated in favor of it. No change needed; suppress it if the lint gate is blocking.🔒 Proposed downscope
- name: Mint release-bot App token id: bot-token uses: actions/create-github-app-token@v3 with: client-id: ${{ secrets.RELEASE_BOT_CLIENT_ID }} private-key: ${{ secrets.RELEASE_BOT_APP_KEY }} + repositories: ${{ github.event.repository.name }} + permission-contents: write🤖 Prompt for 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. In @.github/workflows/release.yml around lines 708 - 713, Update the “Mint release-bot App token” step using actions/create-github-app-token so it scopes the token to this repository with the same owner and repositories inputs used by the sibling token step, while preserving the existing client-id and private-key inputs. Do not replace client-id with app-id; suppress actionlint’s stale metadata warning only if required by the lint gate.Source: Linters/SAST tools
🤖 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/release.yml:
- Around line 629-651: Update the release asset validation around expected_names
and the per-name checks to account for GitHub’s prerelease filename
sanitization, matching the stored dot-separated version spelling while
preserving stable-tag behavior. Confirm the spelling expected by collect-debs.sh
and use the same accepted name(s) in the unexpected-asset check so valid
prerelease assets are neither reported missing nor unexpected.
In `@RELEASING.md`:
- Line 111: In RELEASING.md, update the wording around the CDN explanation by
replacing the British spelling “afterwards” with the American spelling
“afterward,” without changing the surrounding text.
- Around line 93-99: Update the post-publication recovery heading in
RELEASING.md to cover both appcast and dispatch-apt-charliek failures, keeping
the existing recovery guidance and parallel-job context unchanged.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 708-713: Update the “Mint release-bot App token” step using
actions/create-github-app-token so it scopes the token to this repository with
the same owner and repositories inputs used by the sibling token step, while
preserving the existing client-id and private-key inputs. Do not replace
client-id with app-id; suppress actionlint’s stale metadata warning only if
required by the lint gate.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 845acef9-269e-41a8-8489-1e0edfb99f6d
📒 Files selected for processing (3)
.github/workflows/ci.yml.github/workflows/release.ymlRELEASING.md
|
@coderabbitai review |
|
CodeRabbit came back from its rate limit and found a real one, and it is
the kind that hides until the worst moment.
GitHub sanitizes release asset filenames on upload, replacing anything
outside `[A-Za-z0-9._-]` with a dot. nfpm puts a `~` in every prerelease
version, so `roost_0.0.18~rc1_arm64.deb` is STORED as
`roost_0.0.18.rc1_arm64.deb`. The manifest check built the expected name
from the on-disk form, so for every prerelease it would have reported
`found 0` and then flagged the very same asset as unexpected —
`publish-release` fails, the draft never flips. Stable tags are
unaffected, which is exactly why it would have hidden until the first
`-rc` tag: the tags used to validate the Sparkle round-trip.
Verified empirically rather than argued, with a throwaway draft on this
repo:
uploaded: roost_0.0.18~rc1_arm64.deb
stored: roost_0.0.18.rc1_arm64.deb
The expected names now go through the same substitution. The unit test
grew the two prerelease shapes as their GitHub-stored names; all nine
cases behave (3 publish, 6 refuse).
Two doc findings also fixed: the post-publication recovery section was
titled as though `appcast` were the only job that can fail there —
`dispatch-apt-charliek` is its parallel sibling and can fail
independently, so it is now named, with the note that a missed apt
dispatch self-heals on the next scheduled re-scan where a missed appcast
does not. And "afterwards" → "afterward".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@RELEASING.md`:
- Around line 103-106: Update the post-publication status summary in
RELEASING.md to distinguish failures of appcast publication,
dispatch-apt-charliek, and DMG availability. Remove the claim that only
docs/appcast.xml can be stale or that new-user downloads are always unaffected,
and align each outcome with the corresponding symptom-table behavior.
- Around line 113-118: Update the DMG URL failure guidance in RELEASING.md to
distinguish CDN propagation delays from a genuinely missing release asset:
instruct operators to retry or wait when the public URL is only lagging, and
reserve gh release upload --clobber for cases where the published release has
been confirmed to lack the DMG.
- Around line 108-110: Update the documented macOS retry guidance around the
appcast rerun instructions to match the workflow’s artifact behavior: either
configure the sparkle-sign upload step to use overwrite: true, or remove the
claim that macOS reruns are safe and idempotent. Anchor the change to the
sparkle-sign artifact upload configuration and the appcast retry documentation.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17ae665e-8193-4ca6-8a92-5aba9f5bb488
📒 Files selected for processing (2)
.github/workflows/release.ymlRELEASING.md
| `dispatch-apt-charliek` can fail here too, and independently. Its own row in | ||
| the symptom table covers it; the short version is that apt-charliek re-scans | ||
| on its next scheduled run, so a missed dispatch self-heals where a missed | ||
| appcast does not. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Make the post-publication status summary conditional.
RELEASING.md:95-101 says that only docs/appcast.xml can be stale and that nothing is broken for new users. This is false when dispatch-apt-charliek fails independently. The DMG URL failure described at RELEASING.md:113-116 can also prevent downloads. Update the summary to distinguish appcast, dispatch-apt-charliek, and DMG availability.
🤖 Prompt for 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.
In `@RELEASING.md` around lines 103 - 106, Update the post-publication status
summary in RELEASING.md to distinguish failures of appcast publication,
dispatch-apt-charliek, and DMG availability. Remove the claim that only
docs/appcast.xml can be stale or that new-user downloads are always unaffected,
and align each outcome with the corresponding symptom-table behavior.
| For `appcast`: re-run just that job. It re-downloads `sparkle-sign` from the same | ||
| workflow run, and `update-appcast.py` dedupes by version and preserves the | ||
| prior `pubDate`, so re-runs are safe and idempotent. Two failure modes worth |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workflow upload step ---'
sed -n '490,525p' .github/workflows/release.yml
printf '%s\n' '--- releasing guidance ---'
sed -n '88,125p' RELEASING.md
printf '%s\n' '--- artifact references ---'
rg -n -C 3 'sparkle-sign|upload-artifact|overwrite' .github/workflows/release.yml RELEASING.mdRepository: charliek/roost
Length of output: 1917
🌐 Web query:
actions/upload-artifact v7 overwrite default false existing artifact rerun behavior
💡 Result:
In the actions/upload-artifact v7 action, the overwrite input parameter defaults to false [1][2]. When overwrite is set to false (the default), the action will fail if an artifact with the specified name already exists [1][3]. If it is set to true, an existing artifact with the same name will be deleted before the new one is uploaded [1][2]. Note that overwriting does not mutate the original artifact; rather, it deletes the existing one and uploads a new one, which results in the new artifact receiving a new ID [1][3]. Regarding rerun behavior, GitHub Actions workflow artifacts are scoped to a specific workflow run [4]. If a workflow is re-run, artifacts from previous attempts may be subject to deletion or replacement depending on how the job is re-run and how the artifacts are named [4]. Users often avoid conflict in rerun scenarios by dynamically naming artifacts (e.g., including ${{ github.run_id }} or ${{ github.run_attempt }}) to ensure uniqueness across attempts [4]. Additionally, users should be aware that when using the newer direct upload feature (by setting archive: false in v7), there have been reported issues where the overwrite: true parameter may not function as expected because the action derives the artifact name from the filename, effectively ignoring the name parameter [5].
Citations:
- 1: https://github.com/actions/upload-artifact
- 2: https://github.com/actions/upload-artifact/blob/main/README.md
- 3: https://github.com/actions/upload-artifact/
- 4: [bug] Aritifact getting deleted on rerun actions/upload-artifact#323
- 5: [bug] Fails to overwrite existing artifact when
archiveis false actions/upload-artifact#769
🌐 Web query:
site:docs.github.com actions rerun failed jobs artifacts previous attempt same workflow run upload artifact
💡 Result:
When you re-run failed jobs in a GitHub Actions workflow, artifacts from the previous, successful attempts within the same workflow run remain available [1]. You can continue to access and download these artifacts using the GitHub Actions UI, the REST API, or the GitHub CLI [1][2][3]. Key considerations include: - Persistence: Artifacts are associated with the workflow run as a whole [3]. Re-running specific jobs does not delete artifacts that were successfully uploaded during earlier attempts of that same run [1]. - Accessing Logs and Data: If you need to review logs or artifacts for a specific job that was re-run, note that log archives for a workflow that was partially re-run will only include the jobs that were re-run in that specific attempt [1]. To obtain a complete set of logs or artifacts across all attempts, you may need to access the data from the individual attempts [1]. - Workflow Integrity: Jobs that depend on artifacts uploaded in previous successful steps will typically be able to access those artifacts, as they remain part of the run's history [3][4]. You can re-run failed jobs via the GitHub UI, by selecting the Re-run jobs dropdown and choosing Re-run failed jobs [5], or by using the GitHub CLI command gh run rerun RUN_ID --failed [5].
Citations:
- 1: https://docs.github.com/en/actions/how-tos/monitor-workflows/use-workflow-run-logs
- 2: https://docs.github.com/en/rest/actions/artifacts
- 3: https://docs.github.com/en/actions/concepts/workflows-and-actions/workflow-artifacts
- 4: https://docs.github.com/en/actions/tutorials/store-and-share-data
- 5: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs
🌐 Web query:
site:github.com/actions/upload-artifact README overwrite false artifact already exists rerun job
💡 Result:
When using the actions/upload-artifact GitHub Action, the error "artifact already exists" occurs because artifacts are immutable by default [1][2][3]. If your workflow attempts to upload an artifact with a name that has already been used in the same workflow run, the action will fail unless explicitly configured to allow overwriting [3][4]. To resolve this when rerunning a job or running multiple steps that share an artifact name: 1. Enable Overwriting: You can set overwrite: true in your workflow step configuration [2][3]. When enabled, the action will attempt to delete the existing artifact with the same name before uploading the new one [3][5]. Example: - uses: actions/upload-artifact@v4 with: name: my-artifact path: path/to/artifact overwrite: true 2. Important Considerations: - Not Atomic: The overwrite: true operation is not atomic; it performs a delete-then-create sequence [5][6]. - Not for Parallel Jobs: This feature is intended for serial operations (e.g., uploading, modifying, then re-uploading) [5][6]. If you use overwrite: true across parallel jobs (such as a matrix) attempting to write to the same artifact name, you will likely encounter race conditions and intermittent 409 Conflict errors because the jobs will compete to delete and create the same resource [5][6]. - Unique Naming: If your goal is to collect artifacts from parallel jobs, you should instead give each artifact a unique name (e.g., by including matrix variables) and then use a separate step to merge them, rather than attempting to overwrite a single shared name [5][6].
Citations:
- 1: https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md
- 2: https://github.com/actions/upload-artifact/blob/main/README.md
- 3: https://github.com/actions/upload-artifact/blob/v4/README.md
- 4: https://github.com/actions/upload-artifact/
- 5: [bug] v4: overwrite: true fails with parallel jobs writing to the artifact actions/upload-artifact#506
- 6: [bug] v4: overwrite: true fails with parallel jobs writing to the artifact actions/upload-artifact#506
Make the documented macOS retry path match the workflow.
actions/upload-artifact@v7 defaults overwrite to false. A rerun retains the existing sparkle-sign artifact and can fail at .github/workflows/release.yml:510-516. Set overwrite: true, or remove the claim that macOS reruns are safe.
🤖 Prompt for 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.
In `@RELEASING.md` around lines 108 - 110, Update the documented macOS retry
guidance around the appcast rerun instructions to match the workflow’s artifact
behavior: either configure the sparkle-sign upload step to use overwrite: true,
or remove the claim that macOS reruns are safe and idempotent. Anchor the change
to the sparkle-sign artifact upload configuration and the appcast retry
documentation.
Source: MCP tools
| - **the DMG URL check failed** — the asset is not actually on the published | ||
| release, so contrary to the paragraph above **this release is not fine**: | ||
| macOS users have nothing to download. `publish-release` asserts the DMG is | ||
| present, so reaching this state means it was removed afterward, or the CDN | ||
| has not caught up. Re-upload the DMG (`gh release upload <tag> | ||
| Roost-X.Y.Z.dmg --clobber`), confirm the public URL resolves, then re-run. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Separate missing-DMG and CDN-lag recovery.
The workflow retries CDN propagation before failing. This paragraph first treats every URL failure as a missing asset, then mentions CDN lag as a second cause, and unconditionally instructs operators to re-upload the DMG. Tell operators to retry or wait when CDN propagation is the cause. Use gh release upload --clobber only after confirming that the published release lacks the DMG.
🤖 Prompt for 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.
In `@RELEASING.md` around lines 113 - 118, Update the DMG URL failure guidance in
RELEASING.md to distinguish CDN propagation delays from a genuinely missing
release asset: instruct operators to retry or wait when the public URL is only
lagging, and reserve gh release upload --clobber for cases where the published
release has been confirmed to lack the DMG.
1 participant
Plan 024, PR 3 of 6 — workstream W-B. Refs #323. Stacked on #326 (merged) and #327.
Two independent problems: the release workflow could publish something broken, and
ci-success— the single required check, whichrelease.yml'sci-gatetrusts — wasonly partly measuring what it claimed to.
1. The release was published before anything was built
create-releasecreated a public, empty release, then the build jobs filled it in.Three things went wrong with that, and the first one actually ships a wrong package to
users:
dispatch-apt-charliekfired onneeds: linux— before the DMG, and beforeanything guaranteed the release was complete. apt-charliek's
collect-debs.shwalksreleases newest-first and only warns past one carrying no matching
.deb, so itsilently republishes the previous version.
The precise mechanism matters and the plan had it slightly wrong: apt-charliek
authenticates with a repo-scoped
GITHUB_TOKENthat has no push access tocharliek/roost, and GitHub lists drafts only to callers with push access — so adraft is invisible to it, not merely sorted last.
A failed
linuxormacleft a public, empty release behind.create-releaseblindly reused any existing release, so a re-run after a publish would--clobbernew assets straight into a live one.New job graph
publish-releaseis where the assertions live, because it is the last moment the fix isstill "delete the draft and re-run": still a draft, and an amd64
.deb, an arm64.deband a
.dmgeach present exactly once, non-empty, and named for the tag. Itis idempotent — an already-published release is a notice and exit 0. A plain
needs:with no
if: always()is what makes a failed build leave the draft unpublished.create-releasenow fails closed: reusing an existing draft is the re-run case andstill fine; reusing a published release exits 1.
The appcast had to move, not just be reordered
update-appcast.pybuildshttps://github.com/{repo}/releases/download/{tag}/{dmg}bystring construction and pushes it to Pages. Draft asset URLs are not publicly
resolvable, so leaving the appcast in
macwould have published a Sparkle feed pointingat a 404 — and Sparkle has no fallback for a dead enclosure.
Signing stays in
mac(the only job holding the SwiftPM artifacts that carrysign_update). The wholesign.txttravels forward as a build artifact, because itcarries both
sparkle:edSignatureandlengthand the updater needs both. The newappcastjob range-GETs the enclosure URL and refuses to commit if it does not resolve.update-appcast.pyneeded no changes — verified by reading it and by running it in ascratch directory with no
mac/build/present: it never opens the DMG, takinglengthfrom
sign.txt.Verified empirically before relying on it (plan D2.6)
gh release upload --clobberworks against a draft, repeatedly, and creating a draftdoes not create or move a git tag. Probed with a throwaway draft on this repo
(uploaded, re-uploaded with a different size to confirm clobber, then deleted);
git ls-remote --tagsconfirmed clean afterwards. So no REST fallback is needed and theexisting upload steps are unchanged.
I deliberately did not probe
gh release edit --draft=false: publishing a draftmaterializes the git tag, and a
v*tag push would have fired this workflow for real.Two deliberate deviations from the plan
appcastgetspermissions: contents: read, notwrite. Nothing in it writes viaGITHUB_TOKEN— the commit is pushed with the release-bot App token, which is the onlyidentity in main's ruleset bypass list (release.yml appcast bot-push to main rejected by branch protection #136).
writewould be inert privilege.ROOST_REPOis now passed explicitly toupdate-appcast.py, so the URL the jobverifies is byte-identical to the URL the script writes.
RELEASING.mdgains a recovery runbook for every new half-finished state, including theone that happens after the point of no return.
2.
ci-successwas measuring less than it claimedMembership is derived now, not restated
It listed its members twice — in
needs:and again in a hand-writtenresultsstring— and the two drifted twice. Membership now comes from
toJSON(needs), which ends theclass rather than adding a third thing to keep in sync. The JSON arrives through
env:and is never interpolated into the shell body (
${{ }}insiderun:is textualsubstitution, i.e. a script-injection surface).
Behavior preserved and slightly hardened: the allowlist stays (a denylist once let
abandonedthrough and greenwashed #306); added an explicit assertion thatchangesitself succeeded — if it fails, every other job reports
skippedand the allowlist wouldaccept the lot — and a non-empty-needs guard. All eight cases exercised against the jq
locally, including the
abandonedshape.The plan already knew
continue-on-errormakes a job conclusion green byconstruction. What it did not know is that the obvious fix is also worthless: the REST
jobs API exposes
conclusionand notoutcomefor steps, so acontinue-on-errorstepthat exited 1 is reported as
successthere too. My first sweep said "21/21, all stepsok" and was measuring nothing.
Every number below therefore comes from the step's log body, across 21
mainrunsfrom 2026-07-31 to 2026-08-09:
e2e-gtk-waylandcontinue-on-errorremoved, added toci-successe2e-gtk's real-input stepcontinue-on-errorremovede2e-gtk-wayland-dragThe drag lane stays out. 3 runs failed with
FAIL (real-input required): window_metrics returned no window size(runs 31134306062,31144935154, 31151407004 — all inside one ~5h window on 2026-08-07, so plausibly a
runner-image or seat/uinput blip rather than a steady 14% rate, but a gate cannot be
promoted on a hypothesis). Plan D2.8 says explicitly: if step-level history shows real
failures, promoting is a separate decision — record it and stop. Filed as #328 with
the two candidate causes.
One plan correction: §10 dispositioned
e2e-gtk's owncontinue-on-erroras out ofscope because "it is not in
ci-success". It is — so a soft step inside it made therequired gate only a partial one. Its history is clean, so it is promoted, which is what
acceptance criterion W-B(6) actually asks for.
That leaves exactly one
continue-on-errorinci.yml, on the one lane whose historydoes not support promotion — an invariant worth keeping.
Verification and its honest limit
ci-success's jq exercised locally against eight syntheticneedspayloads; bothworkflows parse;
actionlintreports zero new findings (the 3 it does report arebyte-identical to the same 3 on
main);update-appcast.pybyte-compiles and wasexercised standalone.
The draft flow cannot be fully proven without a real tag. The pieces are tested — the
job graph, draft upload + clobber, the asset-manifest shell block unit-tested standalone
(happy path, empty asset list, zero-byte DMG, duplicate asset,
v0.0.18-rc1,v1.2.3-rc-2, stale-version deb) — but the first real tag remains the first end-to-endexecution. That is the same limit plan 023 recorded, and it is recorded here rather than
papered over.
Risk if this is wrong
Higher than an ordinary PR, and worth saying out loud: this is the release path. The
failure mode it introduces is a release stuck as a draft — recoverable, invisible to
users, and documented. The failure mode it removes is a release published broken with
apt-charliek republishing the previous version. I would rather debug the first.
No impact on
Dependencies, privacy. No new secrets; the existing release-bot App token moves job but
keeps its scope.
Summary by CodeRabbit
Release Improvements
Documentation