Conversation
Pushing a v* tag now gates the tag against the declared version, builds the unsigned bundle and installer, confirms every artifact the release contract names exists, and prepares a draft prerelease pinned to the tagged commit. It attaches nothing, and that is the design rather than a gap. Signing needs the hardware token, which is only exposed to an interactive session on the release workstation, and the release plan requires every Windows download to be signed including a beta. So the workflow does everything that does not need the key and stops. A test asserts the negative property the whole arrangement rests on: no job attaches what it built to a release. That is one line away from being wrong at any time, and wrong in a way no test of the build itself would notice. The tag gate is a script rather than inline YAML so it is testable and can be run before pushing. It shares one version grammar with the updater and with the installer's Windows version fields, so a tag that cannot be published is refused here instead of producing an asset nothing can compare. A tag that disagrees with the version literal is the mistake worth catching early: the release would publish, the installer would install, and the fault would surface later as an update every installed copy declines, because the updater compares the feed's tag against the version compiled into the installer. The draft notes are generated by a script too. Prose with backticks and blank lines inside a YAML block scalar inside a shell heredoc has three levels of quoting to get wrong, and the failure mode is a note that truncates at the first surprise. It refuses a tag that contradicts the version, since the heading names the tag while the instructions name filenames built from the version. Write permission is held only by the drafting job; the job that runs third-party packaging tools has no token that can publish. Re-running a tag refreshes the notes rather than recreating the release, so a signed asset already uploaded against it is not discarded. A workflow_dispatch run rehearses the checks without touching releases. Alpha-OSK has no release automation to copy: its releases are built locally and published by hand. This is new work rather than parity.
34f8b75 to
65732f4
Compare
owenpkent
left a comment
There was a problem hiding this comment.
Reviewed the tag gate, candidate checkout/build, artifact contract, generated draft instructions, and release mutation conditions. Three workflow cases need correction: rehearsing a proposed tag, dispatching from an existing tag, and classifying stable releases. Details and regression suggestions are inline.
This was a static workflow review. No workflow was dispatched, tag pushed, release changed, installer signed, or artifact published during validation.
| with: | ||
| # The version gate reads the working tree, so it has to be the | ||
| # tagged commit rather than a branch tip that has moved since. | ||
| ref: ${{ github.event.inputs.tag || github.ref }} |
There was a problem hiding this comment.
[P2] Rehearse a proposed tag against the selected commit, not the nonexistent tag ref.
The new documentation says workflow_dispatch can rehearse the checks before a tag exists, but the input is also used as the checkout ref here. Entering a proposed vX.Y.Z therefore fails in checkout before check_tag.py can validate the version or the candidate can build.
For dispatch, check out the workflow's selected commit/ref, and pass the input string only to the version gate. Preserve the tagged commit checkout for actual tag pushes. Add coverage for dispatch with a proposed tag that has no Git ref, so the documented rehearsal is exercised.
| needs: candidate | ||
| # Only on a real tag: a dispatch run is a rehearsal of the checks and | ||
| # should not touch releases. | ||
| if: startsWith(github.ref, 'refs/tags/v') |
There was a problem hiding this comment.
[P2] Restrict draft mutation to tag-push events.
workflow_dispatch can run against an existing tag, so github.ref can start with refs/tags/v during a manual rehearsal too. This condition then enables the write-permission job and calls gh release create/edit, contrary to the stated promise that dispatch does not touch releases. The edit path also passes --draft to any existing release it finds.
Require the event to be push as well as requiring a version-tag ref. Add a workflow assertion or event-matrix test for tag push, branch dispatch, and tag dispatch, with only the first permitted to mutate a release.
| gh release edit "$TAG" --draft --title "Offloader $VERSION" \ | ||
| --notes-file release-notes.md | ||
| else | ||
| gh release create "$TAG" --draft --prerelease \ |
There was a problem hiding this comment.
[P2] Derive prerelease status from the validated version.
check_tag.py accepts stable tags such as v1.0.0, but this path always creates a prerelease. Publishing that draft without manually correcting the flag leaves the stable installer outside the updater's /releases/latest feed. GitHub explicitly excludes prereleases from that endpoint.
Only pass --prerelease for beta/RC versions, and define how a rerun updates an existing draft's classification. Cover both a stable version and a prerelease in the workflow/release-helper tests.
Reference: GitHub's latest-release API contract.
Three ways the workflow did not do what it says it does. The dispatch input was used as the checkout ref as well as the version to gate. Entering a proposed vX.Y.Z therefore failed in checkout before check_tag.py could say anything about it, which is the whole documented purpose of the rehearsal. The run now checks out whatever commit it was started from - the tagged commit on a push, the selected branch on a dispatch - and the input reaches only the gate. A tag is a version to validate, not a ref to fetch. The drafting job was guarded on the ref alone. A dispatch can be started against an existing tag, and github.ref is a tag ref then too, so a rehearsal reached the job with contents: write, created or edited the release, and passed --draft to one that had already been published. It now requires the event to be a push as well. --prerelease was passed unconditionally, while check_tag.py accepts stable tags. Publishing v1.0.0 as a prerelease leaves the installer outside GitHub's /releases/latest, which is the feed the updater reads, so every installed copy would go on declining the release meant for them. The classification comes from the version the gate validated, and is set explicitly on both the create and the refresh path, so a rerun corrects an existing draft rather than inheriting whatever the first run chose. check_tag.py writes version and prerelease to GITHUB_OUTPUT itself, so the facts the draft is built from come from the step that validated them rather than a second reading that could disagree. It writes nothing when the gate fails. Tests: the drafting condition evaluated against all three events rather than matched as text, since the defect was a condition that read correctly and was true in a case nobody had enumerated; the checkout ref not being the input and the gate still receiving it; the classification of six versions; and the outputs written on a pass and absent on a refusal.
Summary
The updater in #13 needs a feed to read, and nothing produced one. Pushing a
v*tag now:src/offloader/_version.pybefore spending a packaging run on itwindows-latestIt attaches nothing, deliberately
This is the design, not a gap. Signing needs the hardware token, exposed only to an interactive session on the release workstation, and
docs/release-plan.mdis explicit: signing is mandatory for any Windows release including a beta, and unsigned CI output must never be promoted to a download. So the workflow does everything that does not need the key and stops. The signed installer is uploaded separately, and the draft notes carry that command.test_nothing_it_builds_is_attached_to_a_releaseasserts that property. It is one line away from being wrong at any time, and wrong in a way no test of the build itself would catch.Why the gate is a script
scripts/check_tag.pyrather than inline YAML, so it is testable and can run before pushing a tag. It shares one version grammar with the updater and with the installer's Windows version fields, so a tag that cannot be published (latest,v1.2,v1.2.3-evil) is refused here instead of producing an asset nothing can compare.A tag disagreeing with the version literal is the mistake worth catching early, because it does not look like a failure later: the release publishes, the installer installs, and the fault surfaces as an update every installed copy declines — the updater compares the feed's tag against the version compiled into the installer, so the mismatch only appears as a silently broken update path.
Two bugs I caught before they shipped
<<'PY'and<<EOF) had indented terminators, which never close once YAML dedents the block scalar. Replaced with a one-linepython -cand a script. I syntax-checked everyrun:block withbash -nrather than trusting it.cmd-style^continuations while this project documents PowerShell. Now single-line.scripts/release_notes.pyexists because prose with backticks and blank lines inside a YAML block scalar inside a shell heredoc has three levels of quoting to get wrong, and the failure mode is a note that truncates at the first surprise. It also refuses a tag that contradicts the version, since the heading names the tag while the instructions name filenames built from the version.Other decisions
contents: writeis held only by the drafting job. The job running third-party packaging tools holds no token that can publish.workflow_dispatchrehearses the checks without touching releases, guarded by arefs/tags/vcondition on the drafting job.windows-bundleCI job exactly (Python 3.12, NSIS 3.12.0 via choco), since that combination is already proven on the runner.Not parity
Alpha-OSK has no release automation: releases are built locally with the token and published by hand with
gh release create. There was nothing to port, so this is new work. Noted in the release plan so the next reader does not go looking for a reference implementation.Test plan
tests/test_release_workflow.py+tests/test_check_tag.py— 28 tests: tag trigger, no-asset-upload, draft-is-draft, permissions scoped to the drafting job,--no-sign, gate ordered before the build, dispatch cannot touch releases, notes content, notes refusing a contradictory tagrun:blocks passbash -ncheck_tag.py v0.1.0exits 0,v9.9.9exits 1 naming both versions,latestexits 2ruff check src tests scripts build/windowsclean