feat(validatereleasetag): validate release tag - #69
Conversation
ref: AB#22954
There was a problem hiding this comment.
Pull request overview
Adds explicit release-tag validation to the GitHub Actions publish flow so PyPI publishing only proceeds for GA-style tags (vX.Y.Z), aligning the workflow trigger/conditions with tag-based releases.
Changes:
- Broadened workflow tag trigger to
v*(GitHub Actions tag patterns are glob-based). - Restricted the
publishjob to run only on tag pushes (refs/tags/v...), notmasterpushes. - Added a bash validation step that hard-fails publishing when the tag is not exactly
v<major>.<minor>.<patch>.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
.github/workflows/python-tests.yml:104
- The validation hard-codes
masterin both the fetch and the ancestry check. This adds another place to update if the default branch name changes; the workflow already has access to the repo default branch via the event payload.
git fetch origin master
if ! git merge-base --is-ancestor "$GITHUB_SHA" "origin/master"; then
echo "::error::Release tag $TAG does not point to a commit contained in master."
exit 1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
Suppressed comments (3)
.github/workflows/python-tests.yml:12
on.push.tagsuses glob patterns (not regex). Patterns likev[0-9]+\.[0-9]+\.[0-9]+will not match normal SemVer tags (the+and backslashes are treated literally), so tag pushes may never trigger this workflow. Use a broad glob (e.g.v*) and keep strict validation inside the publish jobs.
tags:
- 'v[0-9]+\.[0-9]+\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-beta\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-PR[0-9]+\.[0-9]+'
.github/workflows/python-tests.yml:147
- Publishing jobs should depend on the test/typecheck jobs. Without
needs: [build, pyright], the package can be published even if the matrix tests or pyright fail, and it may run in parallel before quality gates complete.
runs-on: ubuntu-latest
if: >
github.event_name == 'push' &&
.github/workflows/python-tests.yml:230
- Publishing jobs should depend on the test/typecheck jobs. Without
needs: [build, pyright], the package can be published even if the matrix tests or pyright fail, and it may run in parallel before quality gates complete.
runs-on: ubuntu-latest
if: >
github.event_name == 'push' &&
This workflow installs Python dependencies, runs tests, and publishes packages for stable, beta, and PR preview releases.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Suppressed comments (4)
.github/workflows/python-tests.yml:12
on.push.tagspatterns are treated as glob patterns (not regex). The current patterns include+and\.which won’t match real tags likev1.2.3, so tag pushes may never trigger this workflow. Use a glob pattern (and keep the bash regex validation for strictness).
branches: [master]
tags:
- 'v[0-9]+\.[0-9]+\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-beta\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-PR[0-9]+\.[0-9]+'
.github/workflows/python-tests.yml:86
- Publish jobs no longer depend on the test/lint matrix, so a tag push can publish even if
build/pyrightfailed (or while they’re still running). Addneeds: [build, pyright]so publishing is gated on CI.
name: "Publish Stable"
runs-on: ubuntu-latest
if: >
github.event_name == 'push' &&
.github/workflows/python-tests.yml:229
- Same gating issue as
publish-stable: withoutneeds, preview publishing can run even if tests/lint failed. Addneeds: [build, pyright].
publish-preview:
name: "Publish PR Preview"
runs-on: ubuntu-latest
if: >
.github/workflows/python-tests.yml:146
- Same gating issue as
publish-stable: withoutneeds, beta publishing can run even if tests/lint failed. Addneeds: [build, pyright].
publish-beta:
name: "Publish Beta"
runs-on: ubuntu-latest
if: >
Refactor Python dependency installation and version handling.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (13)
Previously missed (4) — in code that hasn't changed since the last review.
.github/workflows/python-tests.yml:12
on.push.tagspatterns are written like regex (use of+and escaped\.), but GitHub Actions tag filters use glob patterns. As written, tag pushes likev1.2.3/v1.2.3-beta.1/v1.2.3-PR12.1may not trigger this workflow at all.
tags:
- 'v[0-9]+\.[0-9]+\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-beta\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-PR[0-9]+\.[0-9]+'
.github/workflows/python-tests.yml:115
- Using
GITHUB_SHAfor tag ancestry checks can break for annotated tags (SHA may be the tag object, not the commit). Resolve the commit the tag points to before callinggit merge-base.
This issue also appears on line 175 of the same file.
if ! git merge-base --is-ancestor "$GITHUB_SHA" "origin/master"; then
.github/workflows/python-tests.yml:309
- PR preview tags (
vX.Y.Z-PR...) are not a PEP 440 version, and this repo usessetuptools_scmfor versioning. Building/publishing without overriding the version is likely to fail or produce a version PyPI rejects. SetSETUPTOOLS_SCM_PRETEND_VERSIONfor preview builds (similar to the dryrun workflow).
run: |
python -m build
.github/workflows/publish_nuget_dryrun.yml:115
- Using
GITHUB_SHAfor tag ancestry checks can break for annotated tags (SHA may be the tag object, not the commit). Resolve the commit the tag points to before callinggit merge-base.
This issue also appears on line 175 of the same file.
if ! git merge-base --is-ancestor "$GITHUB_SHA" "origin/master"; then
.github/workflows/python-tests.yml:84
publish-stablecan run even if thebuild/pyrightjobs fail because it no longer declaresneeds. This can publish a broken release despite CI failures.
name: "Publish Stable"
runs-on: ubuntu-latest
.github/workflows/python-tests.yml:145
publish-betacan run even if thebuild/pyrightjobs fail because it has noneeds. This can publish a broken beta despite CI failures.
name: "Publish Beta"
runs-on: ubuntu-latest
.github/workflows/python-tests.yml:228
publish-previewcan run even if thebuild/pyrightjobs fail because it has noneeds. This can publish a broken preview despite CI failures.
name: "Publish PR Preview"
runs-on: ubuntu-latest
.github/workflows/python-tests.yml:175
- Using
GITHUB_SHAfor tag ancestry checks can break for annotated tags (SHA may be the tag object, not the commit). Resolve the commit the tag points to before callinggit merge-base.
if ! git merge-base --is-ancestor "$GITHUB_SHA" "origin/develop-beta"; then
.github/workflows/publish_nuget_dryrun.yml:12
on.push.tagspatterns are written like regex (use of+and escaped\.), but GitHub Actions tag filters use glob patterns. As written, tag pushes likev1.2.3/v1.2.3-beta.1/v1.2.3-PR12.1may not trigger this workflow at all.
tags:
- 'v[0-9]+\.[0-9]+\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-beta\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-PR[0-9]+\.[0-9]+'
.github/workflows/publish_nuget_dryrun.yml:145
publish-betacan run even if thebuild/pyrightjobs fail because it has noneeds. This can publish a broken beta despite CI failures.
name: "Publish Beta"
runs-on: ubuntu-latest
.github/workflows/publish_nuget_dryrun.yml:228
publish-previewcan run even if thebuild/pyrightjobs fail because it has noneeds. This can publish a broken preview despite CI failures.
name: "Publish PR Preview"
runs-on: ubuntu-latest
.github/workflows/publish_nuget_dryrun.yml:175
- Using
GITHUB_SHAfor tag ancestry checks can break for annotated tags (SHA may be the tag object, not the commit). Resolve the commit the tag points to before callinggit merge-base.
if ! git merge-base --is-ancestor "$GITHUB_SHA" "origin/develop-beta"; then
.github/workflows/publish_nuget_dryrun.yml:5
- This new workflow duplicates
.github/workflows/python-tests.yml(samename: Artesian SDK Python, same triggers, and real PyPI publish steps). If both workflows exist, tag pushes will run/publish twice, increasing CI load and risking failed or duplicated releases.
name: Artesian SDK Python
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/python-tests.yml:12
on.push.tagsuses regex-like patterns (e.g.+and\.). GitHub Actions tag filters are glob patterns, so these strings won’t match intended semver tags and can prevent the workflow from triggering on releases. Easiest: trigger on anyv*tag and rely on the job-level bash validations to enforce the exact formats.
branches: [master]
tags:
- 'v[0-9]+\.[0-9]+\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-beta\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+-PR[0-9]+\.[0-9]+'
.github/workflows/python-tests.yml:84
publish-stableno longer depends onbuild/pyright(previously the publish job hadneeds). As written, publishing can start (and succeed) even if tests/type-check fail, because jobs run in parallel by default.
publish-stable:
name: "Publish Stable"
runs-on: ubuntu-latest
.github/workflows/python-tests.yml:145
publish-betais not gated onbuild/pyright. This allows beta artifacts to be published even when CI fails.
publish-beta:
name: "Publish Beta"
runs-on: ubuntu-latest
.github/workflows/python-tests.yml:228
publish-previewis not gated onbuild/pyright, so preview packages can publish even if tests/type-check fail.
publish-preview:
name: "Publish PR Preview"
runs-on: ubuntu-latest
.github/workflows/publish_nuget_dryrun.yml:5
- This workflow file name implies a NuGet dry-run, but it is a full Python build/test + PyPI publish workflow (and has the same
name:as the main workflow). This is confusing in the Actions UI and makes it easy to run/publish the wrong pipeline.
# This workflow will install Python dependencies, run tests and lint with a single version of Python
# For more information see: https://help.github.com/en/actions/language-and-framework-guides/using-python-with-github-actions
name: Artesian SDK Python
.github/workflows/publish_nuget_dryrun.yml:16
- This workflow publishes to PyPI on
pushtags. Since.github/workflows/python-tests.ymlalso publishes on stablevX.Y.Ztags, creating a release tag will trigger two publish attempts for the same version (one will fail, and it’s easy to accidentally publish twice if configurations diverge). If this file is intended as a dry run, it should not run onpushtags.
on:
push:
branches: [master]
tags:
- 'v[0-9]+\.[0-9]+\.[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+b[0-9]+'
- 'v[0-9]+\.[0-9]+\.[0-9]+a[0-9]+\.[0-9]+'
pull_request:
branches: [master]
workflow_dispatch:
.github/workflows/publish_nuget_dryrun.yml:199
- Same
=~regex operator issue in the beta publish jobif:condition; it will not parse in GitHub Actions expressions. Rely on the bash validation step (or use supported functions likecontains()/startsWith()).
if: >
github.event_name == 'push' &&
startsWith(github.ref, 'refs/tags/v') &&
github.ref_name =~ '^v[0-9]+\.[0-9]+\.[0-9]+b[0-9]+$'
.github/workflows/publish_nuget_dryrun.yml:303
- Same
=~regex operator issue in the preview publish jobif:condition; it will not parse in GitHub Actions expressions.
if: >
github.event_name == 'push' &&
startsWith(github.ref, 'refs/tags/v') &&
github.ref_name =~ '^v[0-9]+\.[0-9]+\.[0-9]+a[0-9]+\.[0-9]+$'
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
.github/workflows/validate_tag.sh:6
- With
set -u, calling this script with missing args fails with a generic “unbound variable” error. Add an explicit arg-count check and a usage error so workflow failures are actionable.
set -euo pipefail
TAG="$1"
TYPE="$2"
.github/workflows/python-tests.yml:269
- "Validate next minor version" compares only major/minor and can incorrectly accept tags like
v4.3.5b1even though the error message/documentation expects patch0(".0bN"). Enforcepatch == 0(and/or compare full X.Y.Z) so the policy is actually validated.
MASTER_MAJOR=$(echo "$MASTER_VERSION" | cut -d. -f1)
MASTER_MINOR=$(echo "$MASTER_VERSION" | cut -d. -f2)
BETA_MAJOR=$(echo "$BETA_VERSION" | cut -d. -f1)
BETA_MINOR=$(echo "$BETA_VERSION" | cut -d. -f2)
EXPECTED_MAJOR="$MASTER_MAJOR"
EXPECTED_MINOR="$((MASTER_MINOR + 1))"
if [[ "$BETA_MAJOR" != "$EXPECTED_MAJOR" ||
"$BETA_MINOR" != "$EXPECTED_MINOR" ]]; then
echo "::error::Beta version $BETA_VERSION is not the next minor version after master GA $MASTER_VERSION."
echo "::error::Expected: ${EXPECTED_MAJOR}.${EXPECTED_MINOR}.0bN"
exit 1
fi
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/python-tests.yml:460
- Preview tag parsing uses
sedsubstitutions that do not fail when the tag doesn't match; in that casePR_NUMBER/ITERATIONbecome the full tag string and the workflow attempts to checkoutrefs/pull/<tag>/merge(guaranteed failure). Parse + validate in one step and hard-fail with a clear error before checkout.
TAG="${GITHUB_REF_NAME}"
PR_NUMBER=$(echo "$TAG" | sed -E 's/^v[0-9]+\.[0-9]+\.[0-9]+a([0-9]+)\.[0-9]+$/\1/')
ITERATION=$(echo "$TAG" | sed -E 's/^v[0-9]+\.[0-9]+\.[0-9]+a[0-9]+\.([0-9]+)$/\1/')
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/python-tests.yml:460
- The preview tag parsing uses
sedsubstitutions that return the original tag when the regex doesn’t match. That can produce a non-numericPR_NUMBER, causing checkout to fail before the later tag validation step runs (poor error signal). Parse/validate in one step and fail with an explicit::error::if the tag doesn’t match the expected preview format.
TAG="${GITHUB_REF_NAME}"
PR_NUMBER=$(echo "$TAG" | sed -E 's/^v[0-9]+\.[0-9]+\.[0-9]+a([0-9]+)\.[0-9]+$/\1/')
ITERATION=$(echo "$TAG" | sed -E 's/^v[0-9]+\.[0-9]+\.[0-9]+a[0-9]+\.([0-9]+)$/\1/')
.github/workflows/validate_tag.sh:6
- With
set -u, calling this script with missing arguments fails with an unhelpful “unbound variable” error. Add an explicit usage/argc check (like validate_branch.sh) so failures are reported as a GitHub Actions error annotation.
TAG="$1"
TYPE="$2"
.github/workflows/python-tests.yml:8
on.push.tagsuses glob matching. This pattern only triggers for tags that happen to look like 3 dotted segments, so malformed tags (which this PR aims to validate) may not trigger the workflow at all. Use a broad trigger (e.g.v*) and rely on the job-level validation scripts to fail with a clear error.
tags:
- "v[0-9]*.[0-9]*.[0-9]*"
pull_request:
.github/workflows/python-tests.yml:16
- Top-level workflow permissions are very broad (
contents/statuses/checks/id-token: write) and are inherited by jobs that don’t override them (e.g. publish-stable/beta). This increases blast radius unnecessarily; prefer least-privilege defaults and grant write permissions only on the specific jobs/steps that need them.
permissions:
id-token: write
contents: write
statuses: write
checks: write
There was a problem hiding this comment.
🔵 Needs a closer look
The workflow currently references non-existent artifact action versions and has needs wiring that will skip key reporting jobs in normal CI runs.
Review details
Suppressed comments (5)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/python-tests.yml:144
- Because
publish-previewis skipped for normal branch/PR runs, having it inneedscauses this job to be skipped as well. If the report should run for regular CI, droppublish-previewfromneeds(or otherwise make it non-optional).
This issue also appears on line 184 of the same file.
.github/workflows/python-tests.yml:455
- This step extracts
PR_NUMBER/ITERATIONbefore any validation. If the tag doesn't strictly match the preview format, thesedcommands return the full tag string and the checkout ref becomes invalid (or points to an unintended ref). Parse with a regex and fail fast on mismatch.
.github/workflows/validate_tag.sh:6
- With
set -u, calling this script with missing args will fail with an unhelpful "unbound variable" error. Add an explicit argc/usage check before reading $1/$2 so the workflow emits a clear ::error:: message.
TAG="$1"
TYPE="$2"
.github/workflows/python-tests.yml:16
- Workflow-level permissions grant write access to contents/statuses/checks for all jobs that don't override permissions (including publish jobs). Narrow the default to least-privilege and keep write scopes only on the specific jobs that need them.
permissions:
id-token: write
contents: write
statuses: write
checks: write
.github/workflows/python-tests.yml:186
- Because
publish-previewis skipped for normal branch/PR runs, having it inneedscauses this job to be skipped as well. If coverage should run for regular CI, droppublish-previewfromneeds(or otherwise make it non-optional).
needs:
- build
- publish-preview
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The workflow trigger and default permissions configuration introduce avoidable operational noise and overly broad write permissions that should be tightened before merging.
Review details
Suppressed comments (2)
.github/workflows/python-tests.yml:7
on.push.tagspatterns are glob filters (not regex). The current pattern (v[0-9]*.[0-9]*.[0-9]*) still matches many unintended tags (e.g.,v1foo.2bar.3) and adds no real safety because tag validity is already enforced byvalidate_tag.shin the publish jobs. Prefer a simplev*tag trigger and keep strict validation in the jobs.
branches: [master]
tags:
- "v[0-9]*.[0-9]*.[0-9]*"
.github/workflows/python-tests.yml:16
- Workflow-level
permissionsare currently granting broad write access (id-token,contents,statuses,checks) to all jobs, even though most jobs override permissions and publish jobs can operate with read-only repo contents when using an API token. Reduce default permissions to least-privilege and keep write permissions only on the specific jobs that need them.
permissions:
id-token: write
contents: write
statuses: write
checks: write
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The workflow currently references non-existent action versions (actions/*-artifact@v7) and has brittle script invocations that can fail if executable bits aren’t set.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
.github/workflows/python-tests.yml:8
on.push.tagsis a glob pattern, andv[0-9]*.[0-9]*.[0-9]*does not mean “digits only” (e.g.[0-9]*matches a digit followed by any characters). This filter is harder to reason about and doesn’t reliably communicate intent; rely on the job-level tag validators instead.
push:
branches: [master]
tags:
- "v[0-9]*.[0-9]*.[0-9]*"
pull_request:
.github/workflows/python-tests.yml:16
- Top-level workflow permissions are very broad (contents/statuses/checks/id-token write) and apply to all jobs/events; this violates least-privilege and increases blast radius if a job is compromised. Prefer
contents: readglobally and grant write permissions only on the specific jobs/steps that need them.
permissions:
id-token: write
contents: write
statuses: write
checks: write
.github/workflows/python-tests.yml:335
- These scripts are invoked directly; that requires the executable bit to be set in git. To avoid workflow failures due to file mode differences, invoke them explicitly via
bash.
run: .github/workflows/validate_tag.sh "$GITHUB_REF_NAME" beta
- name: "Validate release commit is contained in develop-beta"
shell: bash
run: .github/workflows/validate_branch.sh "develop-beta"
.github/workflows/python-tests.yml:475
- This script is invoked directly; that requires the executable bit to be set in git. To avoid workflow failures due to file mode differences, invoke it explicitly via
bash.
- name: "Validate PR Preview tag"
shell: bash
run: .github/workflows/validate_tag.sh "$GITHUB_REF_NAME" preview
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
…/ARKlab/Artesian.SDK-Python into feature/22954-ValidateReleaseTag
There was a problem hiding this comment.
🔵 Needs a closer look
The workflow’s tag trigger pattern won’t run on PR preview tags and the workflow-level permissions are overly broad.
Review details
Suppressed comments (2)
.github/workflows/python-tests.yml:16
- Workflow-level permissions are overly broad (
id-token: write,contents: write,statuses: write,checks: write) and apply to all jobs by default, increasing blast radius if any job is compromised. Set a minimal workflow default (e.g.contents: read) and keep elevated permissions only on the specific jobs that require them.
permissions:
id-token: write
contents: write
statuses: write
checks: write
.github/workflows/python-tests.yml:7
on.push.tagsfilter won’t match PR preview tags likev4.3.0a69.1(extra.1segment), so the workflow won’t run for preview releases. Also the current pattern is a glob, not a regex; it’s safer to trigger on allv*tags and rely on the job-levelvalidate_tag.shchecks to gate publish steps.
on:
push:
branches: [master]
tags:
- "v[0-9]*.[0-9]*.[0-9]*"
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
ref: AB#22954