Allow cutting releases from the GitHub Actions UI - #113
Conversation
Adds a workflow_dispatch trigger to CI that takes a version, bumps app/build.gradle, commits it to master, tags that commit and builds the release from the tag -- all in one run. It has to be one run: a tag pushed with GITHUB_TOKEN cannot start a new workflow, so there is no tag-triggered run to hand off to. The new release-context job is now the single source of truth for whether a run is a release and which version it is, replacing the github.ref checks. Unit tests are skipped on the dispatch path since the released code is master's already-tested tip. Pushing a v* tag by hand behaves as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dcxhpqhRzLzw9vYKGCFT5
Commit the version bump as Robin Jacobs <RobinJ1995@users.noreply.github.com> -- the identity already used by hand-made commits here -- instead of github-actions[bot]. This changes the commit author only. The push is still authenticated by GITHUB_TOKEN, so the pusher stays github-actions[bot], and the recursion guard that stops the bump commit from starting a second CI run is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dcxhpqhRzLzw9vYKGCFT5
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35e7c65b96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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" |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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
Image tags (where applicable): on a workflow_dispatch run github.ref is the branch the run was started from and github.sha predates the bump commit, so type=ref and type=sha both mislabelled the published image -- a sha-<old> tag on content built from the bump commit, and a branch tag on a release image when dispatched from a non-default branch. Both are now disabled for that event and replaced with values derived from the released commit. Every other event keeps its original tag set exactly. Also moved the explanatory comments out of the metadata-action tags block: they sit inside a YAML block scalar, so they reach the action as literal tag lines, and no PR run exercises the publish jobs to catch it. Push the bump commit and the release tag with a single --atomic push. A half-completed push would leave the default branch bumped but unreleased, and the GITHUB_TOKEN branch push starts no CI run that would surface it. Add the regression test for the bump_versions.py blank-line fix that should have accompanied it. Verified it fails against the old \s*$ pattern and passes against the new one. Where the tools tests were invoked by filename or not at all, CI now discovers them, so a new tools test is picked up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dcxhpqhRzLzw9vYKGCFT5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fed30c2a96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # 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 |
There was a problem hiding this comment.
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 👍 / 👎.
Restrict dispatch releases to the default branch. GitHub picks the workflow definition from the ref selected in the Run workflow UI, and the checkout only swaps in the default branch's contents -- so a release started from another ref would run that ref's release logic with contents:write against the default branch's code. Reject zero-padded version components. Semver forbids them and the Python version tools are permissive, so v01.2.3 would have been committed and tagged before any downstream tooling rejected it. Qualify release checkouts as refs/tags/<tag>. An unqualified ref lets a branch of the same name shadow the tag being released. Fix image provenance where images are published: org.opencontainers.image .revision came from the pre-bump github.sha, and image.version resolved to the branch tag because the equal-priority raw entries made the first one win. The revision label is now derived from the released commit and the release version tag outranks the branch tag. Verify appVersionCode was actually incremented where it is bumped by sed, rather than checking only the version name -- an unmatched pattern would otherwise ship an APK Android refuses as an upgrade. Also correct the recorded tools test count in the tracker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dcxhpqhRzLzw9vYKGCFT5
A dispatch pushes the bump commit and the tag atomically, so a release job failing afterwards left the tag in place and every retry aborted on "tag already exists" -- recoverable only by deleting the tag by hand. An existing tag pointing at the default branch's tip is now treated as a resumable run: the bump, commit and tag step is skipped and the release proceeds from the tag already there. A tag pointing anywhere else is still a hard error. The released commit is recorded in its own step so the image provenance labels still get a SHA on the resume path, where the bump step does not run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dcxhpqhRzLzw9vYKGCFT5
|
Addressed in
Also tightened, from review on the sibling changes: dispatch releases are rejected unless started from Resumable tags. An existing tag pointing at Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26d34c9161
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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 |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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
The retry path equated "resumable" with the tag pointing at the default branch's tip. Any ordinary commit landing there between the failed release and the retry broke that comparison, so the run was refused in exactly the case the retry exists for. Judge the tag by the version its own commit records instead. The release jobs already check out the tag rather than the branch, so a resumed release builds the right code regardless of what the branch has done since, and the image provenance SHA is now resolved from the tag for the same reason. Also updates the operator docs, which still said any already-tagged version is refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017dcxhpqhRzLzw9vYKGCFT5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07ea5df89a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - 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 | ||
| whose tag already exists and points somewhere other than `master`'s tip |
There was a problem hiding this comment.
Fresh evidence in .github/workflows/ci.yml lines 62–72 now validates an existing tag by its stored baseVersionName and explicitly allows master to have moved on, so a matching tag behind the branch tip is resumed rather than refused. This operator documentation still says such a tag is rejected; update it to describe the content-based resume check so recovery instructions match the workflow.
AGENTS.md reference: AGENTS.md:L3-L8
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — fixed in d1ca1f6.
I rewrote that sentence for the earlier stale-docs comment and then changed the check from exact-tip to content-based in the same commit, leaving the wording describing the version it had just replaced. It now says a tag is refused unless its commit already sets baseVersionName to the requested version, that a resume therefore still works once master has moved on past the tag, and that the release builds the tagged commit rather than the branch tip.
Generated by Claude Code
The wording still said a tag away from master's tip is refused, which described the previous exact-tip check rather than the content-based one.
A tag can be created from anywhere, so matching version surfaces alone let a commit that was never on the release branch — and never tested — be published. Require the tag to be reachable from the branch, and to be the newest release: resuming an older one would reuse the current version code and drag the mutable image aliases back to a superseded build. Validate the tagged tree with the branch's copy of the checker rather than the tag's own. This job holds contents: write with a persisted credential, so running a script supplied by the tag would hand that access to anyone able to push a tag.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d5b3ae6f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # 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 tag -l 'v*' | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+[a-z]?$' | sort -V | tail -1)" |
There was a problem hiding this comment.
When the expected sequence advances from v3.0.0e to the stable v3.0.0, GNU sort -V—documented as a natural version sort—orders v3.0.0 before v3.0.0e, so this selects the superseded prerelease as newest. A failed stable release is therefore refused on retry while the older beta remains resumable, defeating the guard against rebuilding superseded tags; compare tags with the repository's prerelease semantics so the unsuffixed version sorts after suffix variants of the same numeric version.
AGENTS.md reference: AGENTS.md:L28-L31
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and a real bug in the guard I added an hour ago. Fixed in e325e7a.
I checked sort -V when I wrote it, but only against inputs where the answer didn't depend on this — v3.0.0d and v3.0.0e sort correctly relative to each other, and v3.0.1 after both, so the ordering looked right. The case that breaks it is exactly the one this repo does at the end of a beta cycle: with v3.0.0d, v3.0.0e and v3.0.0 present, sort -V puts the stable first, so newest came out as v3.0.0e and a failed stable release could not be resumed while the superseded beta still could.
Reproduced before and after:
tags: v3.0.0d v3.0.0e v3.0.0
before: newest = v3.0.0e # stable refused, beta resumable — backwards
after: newest = v3.0.0
The fix maps an unsuffixed version to <version>{ before sorting and strips it afterwards. { is 0x7B, immediately past z, so the stable tag sorts after every letter variant of the same numeric version while sort -V still orders the numeric parts. Full order now: v2.9.9 v3.0.0d v3.0.0e v3.0.0 v3.0.1.
The three sibling repos are unaffected — they reject letter suffixes outright and only ever compare strict x.y.z, where plain sort -V is right.
Generated by Claude Code
| 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." |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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
Serialise release runs into one concurrency group. A dispatch runs with a branch ref and a tag push with a tag ref, so they landed in different groups and could interleave; the newest-tag check is only a snapshot, and a tag pushed just after it let the older run finish last and take the mutable image aliases with it. Refuse to resume a tag that already has a published GitHub Release. Resuming exists for a run that failed before publishing; once the release is out, rebuilding would silently replace its signed artefacts with a build that need not be identical.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e325e7a0ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/releases/tags/$TAG" || echo 000)" | ||
| if [ "$release_state" = "200" ]; then |
There was a problem hiding this comment.
The newly added lookup still fails open: any response besides 200—including 401/403, 5xx, or a transport failure converted to 000 by || echo 000—is treated as proof that the release is absent. If a completed release is redispatched during such a transient failure, the workflow proceeds and can overwrite its artifacts once API access recovers; curl --help all confirms that --fail is what makes HTTP errors fail. Accept only the expected 404 as “not published” and abort on every inconclusive response.
AGENTS.md reference: AGENTS.md:L37-L44
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and fixed in be38962.
The guard was added specifically to stop an overwrite, and I wrote it so that the most likely way for the check itself to break — a 5xx, an expired token, a dropped connection — silently disabled it. || echo 000 made that worse by converting a transport failure into a value that looked like a clean negative. Testing only the happy 404 and the 200 I was guarding against is what let it through.
Now only 404 counts as "not published". 200 refuses as before, and anything else — including 000 — refuses with the HTTP code in the message, since an inconclusive answer is not evidence the release is absent.
404 -> resume
200 -> refuse (already published)
500 -> refuse (inconclusive)
000 -> refuse (inconclusive)
All four verified against a stubbed API. Same change in all four repos.
I kept -w '%{http_code}' rather than moving to --fail: --fail would collapse 404 and 500 into the same non-zero exit, which is exactly the distinction this needs.
Generated by Claude Code
| # 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 tag -l 'v*' | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+[a-z]?$' \ |
There was a problem hiding this comment.
This scans every repository tag even though the preceding check permits resuming only tags reachable from master. If a syntactically valid higher version exists on an unmerged branch—for example, a manually pushed tag whose release run then fails—the legitimate newest tag on master is permanently reported as superseded and cannot be resumed. Filter the candidates to tags merged into HEAD; git tag -h documents --merged <commit> as printing only merged tags.
AGENTS.md reference: AGENTS.md:L40-L44
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and fixed in be38962.
The two checks disagreed about what counts as a candidate. The ancestry check restricts what may be resumed to tags reachable from master, but the newest-tag scan used git tag -l 'v*', which sees every tag in the repository. So a syntactically valid higher version on an unmerged branch — a hand-pushed tag from an abandoned attempt, say — would mark the real newest release on master as superseded, and no amount of retrying would clear it: the blocking tag isn't a release and is never going to be superseded in turn.
Now git tag -l --merged HEAD 'v*', so both checks agree on the same candidate set. HEAD is master in this job, since the checkout pins it there on dispatch.
Verified: with v0.3.0 on master and a higher v0.9.0 on an unmerged branch, resuming v0.3.0 succeeds — before the fix it was refused as superseded by v0.9.0.
Same change in all four repos.
Generated by Claude Code
The existing-release check treated every non-200 response as proof of absence, so a 5xx, a 401 or a dropped connection reopened the overwrite it was added to prevent. Only 404 now means 'not published'; anything else refuses. Restrict the newest-release scan to tags merged into the release branch. It scanned every tag, so a higher version pushed on an unmerged branch would mark the real newest release as superseded and block resuming it for good.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be38962ae4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| exit 1 | ||
| fi | ||
| if ! git show "$TAG:app/build.gradle" 2>/dev/null \ | ||
| | grep -qE "^def baseVersionName = \"${TAG#v}\"\$"; then |
There was a problem hiding this comment.
When resuming an existing tag whose stored version differs only at the separators—for example, tag v3.0.1 with baseVersionName = "3-0-1"—the unescaped dots interpolated into this ERE match any character, so the provenance guard accepts the mismatched tag, skips tests, and publishes an artifact whose actual version does not match the requested tag. Use a fixed-string comparison or escape the version before matching.
AGENTS.md reference: AGENTS.md:L37-L40
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and fixed in ec72b1b.
The version was interpolated straight into an ERE, so its dots matched any character and a tagged commit recording baseVersionName = "3-0-1" satisfied a request for v3.0.1. Now a literal whole-line comparison with grep -qxF.
tag v3.0.1 vs baseVersionName = "3-0-1"
before: matched (guard bypassed)
after: rejected
tag v3.0.1 vs baseVersionName = "3.0.1" -> still accepted
Contrived on its own, but this guard is what establishes that the tag really is the bump commit for the requested version, and everything downstream — skipping tests, resuming the release — rests on it. It should not be the loosest check in the chain.
The three sibling repos compare VERSION with a shell string equality rather than a regex, so they were never affected.
Generated by Claude Code
| else | ||
| echo "RELEASE_RESUME=false" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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
Order candidate tags by creation date rather than version magnitude. Downgrades are deliberately accepted, so the highest version is not necessarily the latest release: a v1.0.0 cut after v2.0.0 could be tagged by the fresh path and then refused by the resume path, stranding a tag the workflow had just created. This also retires the pre-release sort special-case, which only existed because version magnitude was the wrong ordering to begin with. Run the trusted checker with python3 -I. Copying the checker in was not enough: the tag's tools/ directory still landed first on sys.path, so a tag-supplied tools/pathlib.py executed when the checker imported pathlib — the same tag-controlled execution the copy was meant to prevent. Compare the tagged version literally. It was interpolated into an ERE, so its dots matched any character and a tag recording 3-0-1 satisfied v3.0.1.
A concurrency group holds at most one running and one pending run, and a newly queued run cancels the pending one regardless of cancel-in-progress. Putting every release in one group therefore let a dispatch cancel a queued tag-push release, leaving that tag with no pipeline at all — silently, and worse than the interleaving it was meant to prevent. This restores the previous per-ref grouping. The race it addressed is still open: a dispatch and a tag push land in different groups and can interleave, so a resumed run may finish last and take the mutable image aliases with it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ee2d35271
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # 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 |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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
| -H "Accept: application/vnd.github+json" \ | ||
| "$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/releases/tags/$TAG" || echo 000)" | ||
| case "$release_state" in | ||
| 404) ;; # no release for this tag — the resumable case |
There was a problem hiding this comment.
When two dispatches resume the same failed tag concurrently, both can receive 404 here before either reaches the release action. Fresh evidence beyond the earlier completed-release guard is that the final workflow has no concurrency group or pre-upload recheck, so both builds continue and the second softprops/action-gh-release invocation can replace the first run's identically named signed assets because overwrite_files defaults to true. Serialize resumptions for a tag or make publication refuse overwrites.
AGENTS.md reference: AGENTS.md:L35-L44
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and not fixed — deliberately parked with the repo owner rather than patched.
The check is a snapshot, two concurrent dispatches can both see 404, and there is no concurrency group now because I added one and then reverted it: a shared release group holds at most one running and one pending run, and a third queued run cancels the pending one, so a hand-pushed tag release could be silently dropped. That is worse than the race, so the revert stands.
Context for why this isn't getting another patch. Resume was not part of the original change — it came out of a review comment mid-way — and has never run. Since it went in, every round of review has found a defect in the guard added the round before: version-magnitude ordering, a fail-open API check, mismatched candidate sets, a sys.path escape in the "trusted" checker, and the concurrency group I had to revert. Each fix was correct in isolation; the path now carries six interacting conditions and keeps producing findings in the same family — this one, GHCR state not being checked, and mutable aliases rolling back on a behind-tip resume.
I've put the choice to the repo owner: keep hardening, keep as-is with the limits documented, or drop resume entirely and require deleting the tag and release before a retry. That last option removes all six conditions and closes this finding along with the rest of the family, at the cost of two manual deletions on a path that has never been exercised. It's my recommendation, but it's their call and I'm not making it unilaterally.
Not disputing the finding — it's real, and it will be either fixed or explicitly accepted once that decision lands.
Generated by Claude Code
The guard sat inside the tag-exists branch, so it only ran on a resume. A GitHub Release outlives a deleted tag, and the superseded-tag error told the operator to delete the tag — following that advice landed on the fresh path, which recreated the tag and handed the surviving release to action-gh-release, overwriting the artefacts the guard exists to protect. The check now runs before either path, and the advice says to delete the release as well.
create-release runs before the artefact jobs, so the ordinary failure leaves a GitHub Release behind. The specs listed reachability, recency and version-surface consistency as the resume conditions but not this one, so an operator following them would be refused without knowing why.
2 participants
Adds a
workflow_dispatchtrigger to CI so a release can be cut from GitHub without a local checkout.How it works
Actions → CI → Run workflow, with a version such as
v3.0.1(orv3.0.1afor a beta). A newrelease-contextjob then:^v[0-9]+\.[0-9]+\.[0-9]+[a-z]?$;baseVersionNameand incrementsappVersionCodeinapp/build.gradle;masterand tags the commit;releasejob 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 two version constants.Pushing a
v*tag by hand is unchanged, tests and all.Why it all happens in one run
A tag pushed with
GITHUB_TOKENcannot start a new workflow run, so there is no tag-triggered run to hand off to. Doing the tag and the release in a single run avoids needing a PAT as a repo secret. A consequence is that the bump commit gets no CI run of its own.Notes
release-contextis now the single source of truth for whether a run is a release and which version it is, replacing thestartsWith(github.ref, 'refs/tags/v')checks — on a dispatch rungithub.refis a branch, not the tag.releasejob guards with!cancelled() && !failure() && …. The status functions are load-bearing: on a dispatch rununit-testsis skipped, and a skipped dependency would skip its dependents under the implicitsuccess(). This form tolerates a skipped dependency but still refuses to release ifunit-testsactually ran and failed.tag_name:explicitly instead of letting the action infer it fromgithub.ref.appVersionCodeis incremented because it must increase monotonically or Android refuses the upgrade. This mirrors how thev3.0.0d→v3.0.0ebump was done by hand.Robin Jacobs <RobinJ1995@users.noreply.github.com>. Only the author is the owner — the push is authenticated byGITHUB_TOKEN, so the pusher remainsgithub-actions[bot], and the commit will show as unverified.master,github-actions[bot]will need a bypass entry or the bump commit cannot be pushed. It is currently unprotected, so nothing is needed today.Verification
ci.ymlparses and the job graph was inspected job by job. The bump logic was exercised against a copy ofapp/build.gradle(107→108,baseVersionName→ the requested version) and the validation regex was checked against both valid and invalid inputs, including3.0.1,v1.2,v1.2.3-betaandv1.2.3ab, all correctly rejected.Not yet exercised end to end: the first real dispatch run pushes to
master, so that one is yours to make. A beta version is the cheap way to try it, since betas are published as pre-releases and are not promoted to latest.🤖 Generated with Claude Code
https://claude.ai/code/session_017dcxhpqhRzLzw9vYKGCFT5
Generated by Claude Code