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
185 changes: 179 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,169 @@ on:
tags: [ 'v*' ]
pull_request:
branches: [ master ]
workflow_dispatch:
inputs:
version:
description: 'Version to release, e.g. v3.0.1 — a single trailing letter (v3.0.1a) publishes a pre-release'
required: true
type: string

jobs:
# Single source of truth for "is this a release, and which version is it".
#
# On workflow_dispatch this also bumps app/build.gradle, commits it to master
# and tags that commit, so the release jobs below build the tagged bump commit
# within this same run. It has to happen in one run: a tag pushed with
# GITHUB_TOKEN cannot start a new workflow run, so there is no second,
# tag-triggered run to fall back on. Manually pushing a v* tag still works
# exactly as before, tests and all.
release-context:
runs-on: ubuntu-latest
permissions:
contents: write # push the version bump commit and the release tag
outputs:
release: ${{ steps.context.outputs.release }}
tag: ${{ steps.context.outputs.tag }}
run_tests: ${{ steps.context.outputs.run_tests }}

steps:
- name: Checkout
uses: actions/checkout@v5
with:
# A dispatch can be started from any ref, but releases always come off
# master. fetch-tags so the "already released?" check below can see them.
ref: ${{ github.event_name == 'workflow_dispatch' && 'master' || github.ref }}
fetch-depth: 0
fetch-tags: true

- name: Validate requested version
if: github.event_name == 'workflow_dispatch'
env:
TAG: ${{ inputs.version }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
set -euo pipefail
# The checkout below swaps in master's *contents*, but GitHub already
# picked this workflow's definition from the ref chosen in the Run
# workflow UI. Releasing from anywhere else would run that ref's
# release logic — with contents:write — against master's code.
if [ "$GITHUB_REF" != "refs/heads/master" ]; then
echo "::error::Releases must be dispatched from master, not ${GITHUB_REF#refs/heads/}."
exit 1
fi
if [[ ! "$TAG" =~ ^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)[a-z]?$ ]]; then
echo "::error::'$TAG' is not a valid version. Expected vMAJOR.MINOR.PATCH, optionally with one trailing lowercase letter for a beta — e.g. v3.0.1 or v3.0.1a."
exit 1
fi
# Refuse if a GitHub Release already exists for this version, whether or
# not its tag does. A release outlives a deleted tag, and either path
# would then hand it to action-gh-release, which overwrites by default
# and would replace already-published artefacts with a fresh build.
# Only 404 proves absence; anything inconclusive refuses.
release_state="$(curl -sS -o /dev/null -w '%{http_code}' \
-H "Authorization: Bearer $GITHUB_TOKEN" \
-H "Accept: application/vnd.github+json" \
"$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/releases/tags/$TAG" || echo 000)"
case "$release_state" in
404) ;;
200)
echo "::error::A GitHub Release already exists for $TAG. Delete it (and the tag, if you want to re-cut the same version) before re-running, or choose a version that has not been released."
exit 1 ;;
*)
echo "::error::Could not determine whether $TAG already has a published release (HTTP $release_state). Refusing rather than risk overwriting published artefacts."
exit 1 ;;
esac

# An existing tag means a previous run bumped and pushed but failed
# later on; that is resumable. Judge it by what the tag *contains*,
# not by where master is now — master may have moved on since, and
# the release job checks out the tag rather than the branch anyway.
if git rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Check for a release even when the tag is absent

If a GitHub Release remains after its Git tag is deleted, this condition bypasses the release API check and treats the version as new; notably, the superseded-tag error on line 82 explicitly tells the operator to “Delete the tag and re-dispatch.” The workflow then recreates the tag and softprops/action-gh-release finds the old release, whose documented overwrite_files default is true, replacing the already-published signed APK/AAB assets. This is fresh evidence beyond the previously fixed completed-release case because the final check is scoped only to tags that still exist; check for an existing release on both paths and remove the tag-only deletion advice.

AGENTS.md reference: AGENTS.md:L28-L34

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Correct, and fixed in e368317 and 0d944a4.

This one was self-inflicted: the guard sat inside the tag-exists branch, so it only ran on a resume — while the superseded-tag error I wrote told the operator to delete the tag. Following my own advice landed on the fresh path, which recreated the tag and handed the surviving release to action-gh-release, overwriting exactly the artefacts the guard exists to protect.

The check now runs before either path, so a release is refused whether or not its tag still exists, and the advice says to delete the release too. 0d944a4 documents the prerequisite in AGENTS.md, since create-release runs before the attachment jobs and the ordinary failure therefore leaves a release behind.

Verified all four combinations against a stubbed API:

fresh path  + stale release (200) -> refuse
fresh path  + no release  (404)   -> proceed
resume path + release     (200)   -> refuse
resume path + no release  (404)   -> resume

Generated by Claude Code

# A tag can be created from anywhere. Releases come off master, and
# resuming skips the tests, so an unreachable tag would publish a commit
# that was never on master and was never tested.
if ! git merge-base --is-ancestor "refs/tags/$TAG^{commit}" HEAD; then
echo "::error::Tag $TAG is not reachable from master — releases must come from master."
exit 1
fi
# Only the newest release is resumable. Re-running an older one would
# reuse the current appVersionCode and drag the release attachments
# back to a superseded build.
newest="$(git for-each-ref --merged HEAD --sort=-creatordate \
--format='%(refname:strip=2)' 'refs/tags/v*' \
| grep -E '^v[0-9]+\.[0-9]+\.[0-9]+[a-z]?$' | head -1)"
if [ "$newest" != "$TAG" ]; then
echo "::error::Tag $TAG is not the most recently cut release ($newest) — only the latest release can be resumed. Delete the tag and its GitHub Release, then re-dispatch, or cut a new version."
exit 1
fi
if ! git show "$TAG:app/build.gradle" 2>/dev/null \
| grep -qxF "def baseVersionName = \"${TAG#v}\""; then
echo "::error::Tag $TAG already exists but its commit does not set baseVersionName to ${TAG#v} — pick a version that has not been released."
exit 1
fi
echo "::notice::Tag $TAG already exists and carries version ${TAG#v}; resuming a previous release run without re-tagging."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject redispatches of completed releases

When the latest matching tag already has a successfully published release, this path still labels it a failed-run resume and proceeds to rebuild and upload the same attachment names. The release action's overwrite_files input defaults to true, so an accidental redispatch silently replaces already-published signed APK/AAB files with a new, potentially non-reproducible build rather than refusing the completed version; verify that the GitHub Release or its expected assets are absent/incomplete before enabling resume.

AGENTS.md reference: AGENTS.md:L37-L41

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Agreed, and fixed in e325e7a.

This is the one failure mode in the resume path where the damage is not recoverable by re-running: softprops/action-gh-release overwrites by default, so an accidental redispatch of an already-released version replaces published signed APK/AAB files with a fresh build. Signing is reproducible but the build is not byte-for-byte, so what users already downloaded stops matching what the release serves.

release-context now queries GET /repos/{repo}/releases/tags/{tag} before accepting a resume and refuses on 200, telling the operator to delete the release (keeping the tag) and re-run, or to cut a new version.

I went with "any published release refuses" rather than trying to detect a partial upload. Distinguishing complete from incomplete means hardcoding the expected asset names in the guard, which then goes stale the moment the attachment list changes — and guessing wrong in the permissive direction is exactly the overwrite this is meant to prevent. The cost is that a run which died between creating the release and finishing its uploads needs the release deleted by hand before the retry; that is a visible, reversible step, unlike a silent overwrite.

Resume therefore now covers the case it was built for: the tag is pushed, the release job fails before publishing. Verified both branches with a stubbed API — 404 resumes, 200 refuses before emitting the resume notice.

The same guard is in all four repos.


Generated by Claude Code

echo "RELEASE_RESUME=true" >> "$GITHUB_ENV"
else
echo "RELEASE_RESUME=false" >> "$GITHUB_ENV"
Comment on lines +111 to +112

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject superseded versions before creating their tags

When an unused but older version is dispatched—for example, v3.0.1 while v3.1.0 already exists—this fresh-tag path performs no newest-version check and proceeds to bump master and push the tag. If the subsequent build fails, every retry then enters the existing-tag path and rejects that same tag as superseded, leaving a workflow-created bump and tag that its recovery path cannot resume; if the build succeeds, it instead publishes an out-of-order release. Validate new tags against the newest merged release before mutating the repository.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This one resolved itself when I changed how "newest" is computed, in ec72b1b.

You're right that the two paths disagreed: the fresh path accepted any unused version (downgrades are a deliberate decision here), while the resume path picked the highest version as newest. A v3.0.1 cut while v3.1.0 existed would tag successfully and then be unresumable — a tag the workflow itself had just created, refused by its own recovery path.

Rather than add a newest-version check to the fresh path — which would have rejected the downgrades the owner explicitly wants — the resume path now orders candidates by tag creation date rather than version magnitude. A downgrade is by definition the most recently cut release, so it resumes normally, and the two paths agree without either giving up its intended behaviour.

Verified: with v2.0.0 tagged first and a deliberate v1.0.0 cut after it, resuming v1.0.0 succeeds and resuming v2.0.0 is refused as no longer the latest.

The out-of-order publishing you mention at the end is real and unchanged — a downgrade moves the mutable image and release aliases backward. That is the accepted cost of allowing downgrades, decided before this PR.


Generated by Claude Code

fi

- name: Bump version, commit and tag
if: github.event_name == 'workflow_dispatch' && env.RELEASE_RESUME != 'true'
env:
TAG: ${{ inputs.version }}
run: |
set -euo pipefail
version="${TAG#v}"

# versionCode must increase monotonically or Android refuses the upgrade.
current_code="$(grep -oP '^def appVersionCode = \K[0-9]+' app/build.gradle)"
sed -i -E "s/^def appVersionCode = [0-9]+$/def appVersionCode = $((current_code + 1))/" app/build.gradle

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Verify that versionCode was actually incremented

If the declaration later gains an inline comment, such as def appVersionCode = 107 // Play version, the extraction on line 68 still succeeds but this more restrictive sed pattern performs no replacement; because the subsequent check validates only baseVersionName, the workflow then commits, tags, and releases an APK with the unchanged version code, which Android will reject as an upgrade. Validate the resulting appVersionCode against $((current_code + 1)) before pushing.

AGENTS.md reference: AGENTS.md:L37-L39

Useful? React with 👍 / 👎.

sed -i -E "s/^def baseVersionName = \".*\"$/def baseVersionName = \"${version}\"/" app/build.gradle

# Fail loudly rather than releasing an unbumped build.
grep -qE "^def baseVersionName = \"${version}\"\$" app/build.gradle || {
echo "::error::Failed to set baseVersionName to ${version} in app/build.gradle."
exit 1
}
grep -qE "^def appVersionCode = $((current_code + 1))\$" app/build.gradle || {
echo "::error::Failed to increment appVersionCode to $((current_code + 1)) in app/build.gradle."
exit 1
}
grep -E '^def (appVersionCode|baseVersionName) ' app/build.gradle

# Attribute the bump to the repo owner, matching every other hand-made
# commit here, rather than to the Actions bot.
git config user.name 'Robin Jacobs'
git config user.email 'RobinJ1995@users.noreply.github.com'
git add app/build.gradle
# Re-releasing a version already present in the file is a no-op commit-wise;
# the tag still needs to land on master's tip.
git diff --cached --quiet || git commit -m "$TAG"
git tag -a "$TAG" -m "$TAG"
# --atomic: either both refs land or neither does. A half-push would
# leave master bumped but unreleased, and the GITHUB_TOKEN branch push
# starts no CI run that would make that visible.
git push --atomic origin HEAD:master "refs/tags/$TAG"

- name: Resolve release context
id: context
env:
TAG: ${{ inputs.version }}
run: |
set -euo pipefail
if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then
# Nothing about the code changed, so the tests that already passed on
# master still hold — go straight to building the release.
printf 'release=true\ntag=%s\nrun_tests=false\n' "$TAG" >> "$GITHUB_OUTPUT"

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 dispatched releases on tests for the exact master SHA

When a dispatch is started while the latest master push's CI is still pending—or after it has failed—setting run_tests=false publishes that revision without any successful test result; the dispatch workflow is independent of the push workflow and never verifies the claimed “already-tested tip.” Run tests in this workflow or require a successful test run for the exact pre-bump SHA before pushing the tag.

AGENTS.md reference: AGENTS.md:L43-L45

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Correct on the mechanics, and deliberate.

The dispatch run does not consult master's CI at all, so "already-tested tip" is an assumption rather than something the workflow verifies. Dispatching while the latest master push is still building, or after it has gone red, will release that revision regardless. The "still pending" case is the more likely accident of the two — merge a PR, immediately cut a release, and the push run may not even have started.

This was weighed before implementing. The alternative considered was to require a successful run for the exact pre-bump SHA (query the Actions API for a green ci.yml run on that commit, refuse otherwise), which is roughly what this comment proposes. The repo owner chose the unconditional skip: it is the simplest thing that makes releasing a one-click operation, and on a single-maintainer repo the person pressing "Run workflow" is the same person who knows whether master is healthy. Trading that check away was the explicit decision, not an oversight.

Two things bound the blast radius: the version input is validated and an existing tag is refused, so a mistaken run cannot overwrite a published release; and a bad release can be deleted and re-cut, since the tag is the only durable artefact.

Leaving as-is. If the assumption turns out to bite in practice, the fix is small and self-contained — a step in release-context, before the bump, that requires a successful run for $(git rev-parse HEAD) — and worth revisiting then.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

no.

elif [ "$GITHUB_REF_TYPE" = "tag" ]; then
printf 'release=true\ntag=%s\nrun_tests=true\n' "$GITHUB_REF_NAME" >> "$GITHUB_OUTPUT"
else
printf 'release=false\ntag=\nrun_tests=true\n' >> "$GITHUB_OUTPUT"
fi

unit-tests:
needs: release-context
if: needs.release-context.outputs.run_tests == 'true'
runs-on: ubuntu-latest

steps:
Expand Down Expand Up @@ -44,17 +204,26 @@ jobs:
name: unit-test-coverage
path: app/build/reports/jacoco/jacocoTestReport/

# Build signed APK + AAB on version tags (v*), gated on the unit tests.
# Build signed APK + AAB for a release, gated on the unit tests.
#
# The status functions matter: on a dispatch run unit-tests is *skipped*, and a
# skipped dependency would skip this job too under the implicit success().
# !cancelled() && !failure() drops that implicit check while still refusing to
# release if unit-tests actually ran and failed.
release:
if: startsWith(github.ref, 'refs/tags/v')
needs: unit-tests
if: ${{ !cancelled() && !failure() && needs.release-context.outputs.release == 'true' }}
needs: [ release-context, unit-tests ]
runs-on: ubuntu-latest
permissions:
contents: write # create/attach the GitHub Release on tags

steps:
- name: Checkout
uses: actions/checkout@v5
with:
# On a dispatch the triggering SHA predates the version bump, so build
# the tag release-context just created rather than the default checkout.
ref: refs/tags/${{ needs.release-context.outputs.tag }}

- name: Set up JDK 21
uses: actions/setup-java@v5
Expand Down Expand Up @@ -99,7 +268,9 @@ jobs:
echo "acra_password=${ACRA_PASSWORD}" >> secret.properties

- name: Set release artifact version
run: echo "RELEASE_VERSION=$GITHUB_REF_NAME" >> "$GITHUB_ENV"
env:
TAG: ${{ needs.release-context.outputs.tag }}
run: echo "RELEASE_VERSION=$TAG" >> "$GITHUB_ENV"

- name: Build signed APK and AAB
run: ./gradlew assembleRelease bundleRelease
Expand Down Expand Up @@ -149,7 +320,7 @@ jobs:
- name: Detect release type
id: release_metadata
run: |
if [[ "$GITHUB_REF_NAME" =~ [[:alpha:]]$ ]]; then
if [[ "$RELEASE_VERSION" =~ [[:alpha:]]$ ]]; then
echo "prerelease=true" >> "$GITHUB_OUTPUT"
{
echo "body<<RELEASE_BODY"
Expand All @@ -163,9 +334,11 @@ jobs:
fi

- name: Attach to GitHub Release
if: startsWith(github.ref, 'refs/tags/')
uses: softprops/action-gh-release@v2
with:
# Explicit: on a dispatch run github.ref is a branch, so the tag cannot
# be inferred from it.
tag_name: ${{ needs.release-context.outputs.tag }}
prerelease: ${{ steps.release_metadata.outputs.prerelease }}
append_body: true
body: ${{ steps.release_metadata.outputs.body }}
Expand Down
24 changes: 24 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,30 @@ codebase — older Java alongside newer Kotlin.
Release attachments are named `DistroHopper-<version>.apk`,
`DistroHopper-<version>.aab`, and `DistroHopper-<version>-paranoia.apk`,
where `<version>` is the full version tag, including its leading `v`.
- Releases can also be cut from GitHub: run the **CI** workflow manually
(Actions → CI → Run workflow) and give it a version such as `v3.0.1` or
`v3.0.1a`. The `release-context` job validates the format, refuses a version

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 Document the same-tip tag resume exception

When the requested tag already points at the current master tip, .github/workflows/ci.yml lines 65–73 explicitly resumes the release instead of refusing it, so this newly added operator documentation is stale and conceals the recovery path introduced by the workflow. Amend this sentence to explain that only tags pointing elsewhere are refused and that a same-tip tag resumes a failed release.

AGENTS.md reference: AGENTS.md:L3-L8

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fair — the docs described the refusal and not the recovery path added in the same change. Fixed in 07ea5df, along with the underlying logic, which turned out to need correcting too.

The resume condition itself was wrong. Comparing the tag against master's tip rejected exactly the case resuming exists for: a release fails after the tag is pushed, an ordinary commit lands before anyone retries, and the retry is refused — with the tag still present, so the fresh path refuses it as well. The tag is now judged by what it carries (baseVersionName at the tagged commit must equal the requested version), so a tag on the unbumped tip is refused while a genuine retry succeeds regardless of where master has got to. The released commit is resolved from the tag rather than from HEAD for the same reason.

AGENTS.md now documents that: a tag pointing elsewhere is refused, and a tag already carrying the requested version is treated as a previous run that pushed but failed later, with the release resuming from it. The equivalent sentence in the three sibling repos' specs was stale in the same way and has been updated too.


Generated by Claude Code

whose tag already exists but does not carry that version (a tag whose commit
already sets `baseVersionName` to it is a previous run that pushed but failed
later, and the release resumes from it — judged by what the tag holds, so a
resume still works once `master` has moved on past it, and the release builds
the tagged commit rather than the branch tip; the tag must also be reachable
from `master` and be the newest release, so a resume cannot reuse a superseded
build's `appVersionCode`; and no GitHub Release may exist for the tag yet — a
release that outlived a failed run, or its deleted tag, must be deleted before
retrying, since the release action overwrites its attachments by default), bumps `baseVersionName` and
increments
`appVersionCode` in `app/build.gradle`, commits that to `master`, tags the
commit (authored as `Robin Jacobs <RobinJ1995@users.noreply.github.com>` — the
same identity as hand-made commits here, and `git config user.*` sets author,
committer and tagger alike; the *push* is still authenticated by
`GITHUB_TOKEN`, so the pusher remains `github-actions[bot]` and the commit is
unsigned), and the release job builds from that tag in the same run. Unit
tests are skipped on this path — the code being released is master's already-tested
tip, and the bump commit touches only the version constants. Pushing a tag by
hand still runs the tests first, unchanged. Note that if branch protection is
ever enabled on `master`, `github-actions[bot]` needs a bypass entry or the
bump commit cannot be pushed.
- Tagged CI releases build the normal signed APK/AAB plus a best-effort
signed `-paranoia` APK (`./gradlew clean assembleRelease -PparanoiaBuild=true`):
it appends `-paranoia` to `versionName`, forces `BuildConfig.ACRA_CONFIGURED`
Expand Down
Loading