Skip to content

Automate miner release versioning - #523

Merged
n13 merged 6 commits into
mainfrom
ci/automated-miner-releases
Aug 14, 2026
Merged

n13 merged 6 commits into
mainfrom
ci/automated-miner-releases

Conversation

@n13

@n13 n13 commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Replaces the manual version-entry miner release with the same automated flow as quantus-cli / chain / quantus-miner:

  • create_miner_release_proposal.yml (new): dispatch with patch/minor/major/custom. Computes the next version from max(latest miner-v* tag, miner-app/pubspec.yaml) — so a manually pre-bumped pubspec can never be silently downgraded — bumps the pubspec, and opens a PR labeled miner-release-proposal (labels auto-created on first run). Custom versions below the current version are rejected; an already-existing tag is rejected via a remote ls-remote check.
  • publish_miner_release.yml (new): when that PR merges — checks out merge_commit_sha directly, verifies pubspec matches the PR-title version, tags the merge commit miner-vX.Y.Z, then builds and releases pinned to that verified merge SHA (never a bare tag name, which checkout would resolve branch-first, letting a same-named branch shadow the release). The GitHub release is published with --notes-start-tag pinned to the previous miner tag so generated notes don't diff against wallet-v* tags. The whole run is idempotent: re-running after a partial failure skips the existing tag (if it points at the merge commit) and re-uploads assets to the existing release instead of dying. A concurrency group serializes publish runs.
  • create_miner_build.yml: now build-only and reusable (workflow_call with a ref input); manual dispatch still available for test builds. The old in-workflow release job (deprecated actions/create-release@v1) is removed.

Notes

  • The proposal uses the PAT_TOKEN secret for gh pr create so CI triggers on the release PR (same as ADMIN_PAT in the other repos). If it has expired, the proposal job fails at PR creation and the secret needs rotating.
  • main pubspec is at 0.6.0 while the latest tag is miner-v0.5.0, so a patch dispatch releases miner-v0.6.1. To release exactly 0.6.0, dispatch with custom = 0.6.0.

Test plan

  • actionlint clean on all three miner workflows
  • 33 local tests run against the actual scripts extracted from the workflow YAML: version math (incl. pubspec-ahead / tag-ahead / multi-digit sort -V cases), custom-version validation and downgrade rejection, pubspec parsing (incl. +build suffix and malformed input), PR-title tag extraction, previous-tag lookup with wallet tags interspersed, and tag-push idempotency against a bare origin (fresh, re-run same commit, re-run fresh clone, collision at different commit)
  • Full end-to-end run on a disposable repo (n13/miner-release-e2e-test) with the two orchestration workflows byte-identical to this branch (pre-SHA-pinning; that later commit only changes which ref the already-proven checkouts receive) and a stub build keeping the exact workflow_call/artifact interface:
    • proposal (patch) → PR ci: Miner version bump to miner-v0.5.1 with correct labels and pubspec bump
    • merge → publish fired, annotated tag created exactly at the PR merge commit, build ran at the release commit, release published with all 3 assets and Full Changelog: miner-v0.5.0...miner-v0.5.1 (wallet tag correctly skipped)
    • full re-run of the publish run → green: "Tag … already points at …; skipping tag creation" + "Release … already exists; re-uploading assets", no duplicate release
    • custom 0.4.0 proposal → rejected in the calculate job (downgrade guard)
    • is_draft=true → PR carried draft-release label and the release was published as a draft

Release proposal workflow computes the next miner-v version
(patch/minor/major/custom) from tags and opens a version-bump PR;
merging it tags, builds all platforms, and publishes the release.

@dewabisma dewabisma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Have you tested the CI workflows? It can be tricky to make CI works without actually run and see it works.

@n13

n13 commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator Author

Yes thats why i did not merge yet ;)

@n13

n13 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

🔍 Review — Automate miner release versioning (CI)

Verdict: 🟡 Approve with non-blocking comments

The design is sound: the release-firing guard is correct (only a merged PR carrying the miner-release-proposal label can tag), the workflow uses pull_request rather than pull_request_target, and untrusted inputs are passed via env vars — so there is no injection or "any-merged-PR-tags-a-release" bug. The remaining items are robustness/idempotency concerns, and the whole post-merge tag→build→release path is explicitly untested in the PR's own test plan.

What it does

  • create_miner_build.yml: converted to build-only + reusable (workflow_call with a ref input), keeping workflow_dispatch for test builds; drops contents: write to contents: read; removes the deprecated actions/create-release@v1 / upload-release-asset@v1 release job.
  • create_miner_release_proposal.yml (new): workflow_dispatch (patch/minor/major/custom) → computes next version from the latest miner-v* tag → seds miner-app/pubspec.yaml → pushes a release/miner-vX.Y.Z branch → opens a PR (via PAT_TOKEN) labeled automated,miner-release-proposal (+draft-release if requested); labels auto-created with gh label create --force.
  • publish_miner_release.yml (new): on merged PR to main labeled miner-release-proposal → extract version from PR title, verify pubspec matches, tag merge_commit_sha, call the reusable build, then gh release create with assets and --notes-start-tag pinned to the previous miner tag.

Strengths

  • Release-firing guard is correct (publish_miner_release.yml:15): github.event.pull_request.merged == true && contains(labels.*.name, 'miner-release-proposal'). When create-tag is skipped, the build and create-release jobs (needs: it) are skipped too, so no unlabeled/unmerged PR can produce a tag or release.
  • Correctly uses pull_request not pull_request_target (publish_miner_release.yml:4). Proposal PRs come from same-repo branches so secrets/write-token are available; forks would get neither, failing safe.
  • No script injection: PR_TITLE (:32), LATEST_TAG/CUSTOM_VERSION/VERSION_TYPE (create_miner_release_proposal.yml:63-66), NEW_VERSION/TARGET_BRANCH (:117-120) are passed via env and quoted, not interpolated into the script body.
  • pubspec-vs-tag verification (publish_miner_release.yml:48-57) is a good safety net if the PR title is edited.
  • Mostly least-privilege: top-level contents: read on publish and build; contents: write/pull-requests: write scoped per job.
  • --notes-start-tag correctly pinned to the previous miner-v* tag with the first-tag edge handled (publish_miner_release.yml:118-121), so generated notes don't diff against wallet-v*.
  • PAT_TOKEN used for gh pr create so the proposal PR actually triggers CI (a GITHUB_TOKEN-created PR would not).

Findings

  1. [non-blocking — top verification gap] create-tag checks out the default PR ref but operates on merge_commit_sha, which may not be fetched. publish_miner_release.yml:24-27 checks out the default pull_request ref (refs/pull/N/merge) with fetch-depth: 0, then :53 runs git show "$MERGE_COMMIT:miner-app/pubspec.yaml" and :66 runs git tag -a "$TAG" "$MERGE_COMMIT". merge_commit_sha (the real merge on main) is a sibling of the test-merge commit and is not in that ref's history, so these can fail with "invalid object name". This is exactly the post-merge step left unchecked in the test plan. Safer: actions/checkout with ref: ${{ github.event.pull_request.merge_commit_sha }} (or explicitly git fetch origin main). Must be validated on a throwaway version before relying on it.

  2. [non-blocking] Version source-of-truth is the latest tag, not pubspec — "patch" can silently downgrade pubspec. create_miner_release_proposal.yml:61-100 derives the next version purely from the latest miner-v* tag, then :135 seds pubspec to that value unconditionally. If pubspec is ahead of the latest tag (the exact situation the PR describes), a patch bump writes a lower version than what's in pubspec. The grep check at :137 only confirms the sed applied, not that the version moved forward. Consider taking max(tag, pubspec) as the base, or refusing a bump that lowers pubspec.

  3. [non-blocking] Proposal's own duplicate-tag guard is likely ineffective — update-version doesn't fetch tags. create_miner_release_proposal.yml:112-113 sets fetch-depth: 0 but omits fetch-tags: true (which calculate-next-version deliberately sets at :41-42). actions/checkout@v4 does not fetch tags by default, so the guard git tag -l | grep -q "^${NEW_VERSION}$" at :126 runs against an empty/partial tag list and won't catch an already-released version. The downstream git push origin "$TAG" still fails safely, but the intended fail-fast is lost. Add fetch-tags: true.

  4. [non-blocking] No idempotency/concurrency handling in publish → no recovery from a partial failure. publish_miner_release.yml:59-67 pushes the tag before build/release. If the build or gh release create fails, re-running the workflow dies at git tag/git push because the tag now exists, with no way to resume. The removed workflow at least probed git ls-remote --tags first. Consider a concurrency: group and tolerating an existing tag (skip-if-present) so the release step can be retried.

  5. [nit] Dispatch inputs interpolated directly into run scripts instead of via env. create_miner_release_proposal.yml:156 (if [[ "${{ github.event.inputs.is_draft }}" == "true" ]]) and :171 (Bump type: ${{ github.event.inputs.version_type }}) break the env-var pattern used everywhere else. Low risk (choice/boolean, dispatch-only), but it's the actionlint expression-injection smell; move to env:.

  6. [nit] No top-level permissions: in create_miner_release_proposal.yml. calculate-next-version (:31) therefore inherits the repo/org default token scope (possibly write-all) though it only reads. Add a top-level permissions: contents: read and keep the per-job elevation.

  7. [nit] Operator instruction in the PR notes is already stale. The notes say "latest tag is miner-v0.4.5… choose minor to release miner-v0.5.0", but miner-v0.5.0 already exists on origin (lightweight tag, commit 11b1d1b9, 2026-06-12). The latest tag is now 0.5.0, so minor yields 0.6.0; following the written instruction would collide.

  8. [nit] sed at create_miner_release_proposal.yml:135 drops any +build suffix (^version:[[:space:]]*.+$version: X.Y.Z). Harmless for current miner-app (pubspec is a bare 0.5.0), but worth knowing if a build number is ever added. Also publish_miner_release.yml:118 matches $TAG with unescaped dots via grep -x (cosmetic; grep -Fx would be literal), and create_miner_build.yml:287 has no trailing newline (pre-existing).

Verification

actionlint clean and local version-math/prev-tag checks are claimed and plausible from the code. The gap is the entire post-merge path — tagging merge_commit_sha, the reusable build handing artifacts to create-release, and gh release create — which the test plan leaves as an unchecked box. Finding #1 lives precisely there. Recommend a full dry run against a disposable version (e.g., a high custom like 0.99.0) on a fork/test repo, confirming the tag lands on the real merge commit and all three assets attach, before trusting this in production. Cross-workflow artifact handoff (reusable build job → create-release's download-artifact) works within a single run, and the artifact names/paths (quantus_miner-{macos,linux,windows}*_macos.zip/*_linux.tar.gz/*_windows.zip) match between create_miner_build.yml:187/243/286 and publish_miner_release.yml:132-134.


🤖 AI-assisted review generated with Claude Code

@n13

n13 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Update: all 8 findings from the review above are now fixed on this branch, and the full flow has been tested for real this time.

Fixes (commit 8b0e6bb3):

  1. create-tag now checks out merge_commit_sha directly — no more "invalid object name" hazard
  2. Version base is max(latest miner tag, pubspec) — a pre-bumped pubspec can't be silently downgraded, and custom versions below current are rejected
  3. Duplicate-tag guard now checks the remote via git ls-remote --exit-code (independent of local tag state)
  4. Publish is idempotent + has a concurrency group: re-runs skip an existing tag pointing at the merge commit and re-upload assets to an existing release
  5. Dispatch inputs (is_draft, version_type) moved to env:
  6. Top-level permissions: contents: read added to the proposal workflow
  7. Stale operator note fixed in the PR body (tag and pubspec are both 0.5.0 now → first run = patchminer-v0.5.1)
  8. grep -Fx for the prev-tag lookup (plus || true so a missing tag can't kill the release via pipefail), asset-existence checks before gh release create, tar glob nit

End-to-end verification on a disposable repo (n13/miner-release-e2e-test) with the two orchestration workflows byte-identical to this branch and a stub build keeping the exact workflow_call + artifact interface:

  • proposal (patch) → PR with correct title/labels/pubspec bump → merge → publish fired → annotated tag created exactly at the PR merge commit → build at the tag → release with all 3 assets and Full Changelog: miner-v0.5.0...miner-v0.5.1 (wallet tags correctly skipped)
  • full re-run of a completed publish run → green (tag skipped, assets re-uploaded, no duplicate release)
  • downgrade custom version → rejected in the calculate job
  • is_draft=true → draft release with all assets

Also 33 local tests run against the actual scripts extracted from the workflow YAML (version math, guards, title parsing, prev-tag lookup, tag idempotency incl. collision at a different commit). What remains untested is only the real Flutter/signing build internals, which this PR doesn't change apart from the trigger/interface conversion — and that interface is exactly what the E2E exercised.

@n13 n13 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Verdict: REQUEST_CHANGES — the release build is not pinned unambiguously to the tag/merge commit.

Blocking finding

  1. [P1] A same-named branch can shadow the release tag and get different code signed and published. In .github/workflows/publish_miner_release.yml:88 and :101, the workflow passes the bare value miner-vX.Y.Z to actions/checkout. Checkout v4 resolves an unqualified ref by testing refs/remotes/origin/<name> before refs/tags/<name>. Therefore, if a branch named miner-vX.Y.Z exists, the build jobs check out that branch even though create-tag correctly created the tag at the merge commit. The macOS job will then build and Developer-ID-sign the branch contents, and create-release will publish those artifacts under the legitimate tag. This bypasses the protected merge/release path for any collaborator able to create a branch. Pass a fully-qualified tag ref (refs/tags/${{ needs.create-tag.outputs.tag }}) at both sites, or output the verified merge SHA and use that SHA for every build/release checkout.

The disposable E2E run is real: the two orchestration workflow files are byte-identical to this head, attempt 2 completed successfully, and miner-v0.5.1 dereferences to the PR merge commit. It did not exercise the branch/tag collision above.

Non-blocking note

  • The PR summary's first-run example is stale after the latest merge from main: both base and head now have miner-app at 0.6.0 while the latest miner tag is 0.5.0, so a patch dispatch computes miner-v0.6.1, not miner-v0.5.1.

Validation

  • Inspected the complete 8e68f1a...4b9888a diff and intervening fix/merge commits.
  • actionlint v1.7.12 — clean for all three changed workflows.
  • dart run melos bootstrap — passed for all four packages.
  • dart run melos exec --concurrency=1 -- 'flutter analyze . --fatal-infos' — all four packages passed.
  • dart run melos exec -- dart format lib test --line-length=120 — passed; 0 files changed.
  • git diff --check — clean.
  • Live GitHub Analyze check — passed.

GitHub does not allow an author to submit a native request-changes review on their own PR, so this is posted as a COMMENTED request-changes opinion.

…d merge SHA

actions/checkout resolves bare refs as branches before tags, so a branch
named miner-vX.Y.Z could shadow the release tag and get different code
signed and published. Build and release now check out the merge commit
SHA that the pubspec verification and tag creation already ran against.
@n13

n13 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Both points addressed in d904d8a8:

P1 (branch shadowing the release tag): create-tag now exports the verified merge_commit_sha as a job output, and both the reusable build call and the create-release checkout are pinned to that SHA instead of the bare tag name. The tag name is now used only for creating the tag itself and for naming the release — never as a checkout ref — so a same-named branch can no longer influence what gets built, signed, or published. This is the stronger of the two suggested fixes (also immune to the tag being moved between jobs).

Stale first-run example: PR body updated — with pubspec at 0.6.0 and latest tag miner-v0.5.0, a patch dispatch releases miner-v0.6.1; releasing exactly 0.6.0 takes a custom dispatch.

SHA checkouts are the same mechanism the E2E already exercised in create-tag (which checked out merge_commit_sha in both green attempts); the change only swaps which ref the two remaining checkout sites receive. actionlint clean.

@n13 n13 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Verdict: APPROVE — the prior release-integrity blocker is fixed, and no blocking findings remain.

The new d904d8a8 commit exports github.event.pull_request.merge_commit_sha from create-tag and passes that verified SHA to both the reusable miner build and the release checkout (publish_miner_release.yml:27, :91, :104). The tag name is no longer used as a checkout ref anywhere in the publish path, so a same-named branch cannot redirect the code that is built, Developer-ID-signed, or packaged. The PR summary's first-run version example is also now accurate for the current 0.6.0 pubspec / miner-v0.5.0 tag state.

Validation:

  • Inspected the complete 4b9888a..d904d8a incremental commit and the full resolved 8e68f1a...d904d8a diff.
  • actionlint v1.7.12 — clean for all three changed workflows, including the new job output/reusable-workflow expression.
  • Exact SHA-wiring assertions — passed: one merge-SHA output, two downstream SHA checkouts, and no remaining tag-output checkout.
  • git diff --check — clean.
  • Live GitHub Analyze check on d904d8a — passed.

No blocking findings remain. GitHub does not allow an author to submit a native approval on their own PR, so this is posted as a COMMENTED approval opinion.

@n13

n13 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Shipping this

@n13
n13 merged commit 3675320 into main Aug 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants