feat(release): add tag/version validation + previous-tag scripts (epic 005 T005) - #42
Conversation
…c 005 T005) Test-first per Constitution I: fixture tests written alongside each script, run against a throwaway git repo (not this repo's own tags). - validate-release-tag.sh: tag format (vMAJOR.MINOR.PATCH) + match against Cargo.toml's [workspace.package].version (FR-010), error names both values on mismatch (SC-006). - previous-release-tag.sh: nearest reachable prior SemVer tag via git describe (FR-011); empty output (not an error) when there is none, for FR-009's first-release fallback to key off. Re-publishing-an-existing-tag rejection is deliberately not here -- it needs the GitHub Release API to check for an existing release, which is T012's concern in release.yml, not something an offline git-only script can determine.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Reviewer's GuideAdds two bash scripts for release tag/version validation and previous-tag resolution, with fixture-based tests and task tracking updates for epic 005 T005. Sequence diagram for validate-release-tag.sh tag/version validationsequenceDiagram
actor Release_workflow
participant validate_release_tag_sh
participant Cargo_toml
Release_workflow->>validate_release_tag_sh: invoke with tag, cargo_toml_path
validate_release_tag_sh-->>Release_workflow: usage (missing args)
note over validate_release_tag_sh: usage is only called on invalid invocation
validate_release_tag_sh->>Cargo_toml: check file exists
validate_release_tag_sh-->>Release_workflow: error (file not found)
validate_release_tag_sh->>validate_release_tag_sh: validate tag regex ^vMAJOR.MINOR.PATCH$
validate_release_tag_sh-->>Release_workflow: error (tag not SemVer)
validate_release_tag_sh->>Cargo_toml: awk read [workspace.package].version
validate_release_tag_sh-->>Release_workflow: error (version not found)
validate_release_tag_sh->>validate_release_tag_sh: build expected_tag from workspace_version
validate_release_tag_sh-->>Release_workflow: error (tag/version mismatch)
validate_release_tag_sh-->>Release_workflow: ok (tag matches workspace version)
Sequence diagram for previous-release-tag.sh previous tag resolutionsequenceDiagram
actor Release_workflow
participant previous_release_tag_sh
participant Git_repo
Release_workflow->>previous_release_tag_sh: invoke with current_tag
previous_release_tag_sh-->>Release_workflow: usage (missing args)
note over previous_release_tag_sh: usage is only called on invalid invocation
previous_release_tag_sh->>Git_repo: git rev-parse --verify refs/tags/current_tag
previous_release_tag_sh-->>Release_workflow: error (tag does not exist)
previous_release_tag_sh->>Git_repo: git describe --tags --match v[0-9]* --abbrev=0 current_tag^
Git_repo-->>previous_release_tag_sh: previous_tag or no match
previous_release_tag_sh-->>Release_workflow: previous_tag on stdout
previous_release_tag_sh-->>Release_workflow: empty stdout if first release
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
owkwo-bot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughAdds release-tag validation and previous-release selection scripts, fixture-based Bash tests, and updated T005 completion notes. Previous-tag lookup now skips prerelease tags while validating strict SemVer releases. ChangesRelease versioning checks
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Up to standards ✅🟢 Issues
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The header comment in validate-release-tag.sh still claims it "resolves the previous release tag" even though that logic now lives in previous-release-tag.sh; consider updating the description to avoid confusion about script responsibilities.
- previous-release-tag.sh uses git describe --match "v[0-9]*", which can match non-SemVer tags like v1; if you truly only want SemVer tags, tightening this to a pattern that enforces MAJOR.MINOR.PATCH would better align with the semantics implied by the validate script.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The header comment in validate-release-tag.sh still claims it "resolves the previous release tag" even though that logic now lives in previous-release-tag.sh; consider updating the description to avoid confusion about script responsibilities.
- previous-release-tag.sh uses git describe --match "v[0-9]*", which can match non-SemVer tags like v1; if you truly only want SemVer tags, tightening this to a pattern that enforces MAJOR.MINOR.PATCH would better align with the semantics implied by the validate script.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
specs/005-ci-release-versioning/tasks.md (1)
89-107: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign T005’s acceptance criteria with its completion note.
T005 still says it must test and reject republishing an existing tag, while the Done note says that behavior is deferred to T012. Remove that requirement from T005 or leave T005 incomplete; otherwise the task status claims completion of work that is intentionally not included.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specs/005-ci-release-versioning/tasks.md` around lines 89 - 107, Update T005’s acceptance criteria and completion note to remove the requirement to test or reject republishing an existing tag, since that behavior belongs to T012 in release.yml. Keep T005 focused on Cargo.toml version/tag validation and previous_release_tag selection, and retain its completed status only for that scope.
🧹 Nitpick comments (1)
.github/scripts/tests/test-previous-release-tag.sh (1)
48-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression case for non-release-format tags.
Insert a tag such as
v1.1.0-rc1betweenv1.1.0andv2.0.0, then assert that the previous tag forv2.0.0remainsv1.1.0. The current fixture would pass even with the broadgit describefilter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/tests/test-previous-release-tag.sh around lines 48 - 69, Extend the fixture around the existing tag setup by adding a non-release-format tag such as v1.1.0-rc1 between v1.1.0 and v2.0.0, then retain or update the v2.0.0 assertion to verify it still returns v1.1.0 rather than the prerelease tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/tests/test-validate-release-tag.sh:
- Around line 16-26: Update assert_ok and assert_fail to create unpredictable
capture files with mktemp inside the existing private $tmp directory instead of
using /tmp/out.$$. Store each generated filename, redirect command output to it,
read it on failure, and remove it afterward.
---
Outside diff comments:
In `@specs/005-ci-release-versioning/tasks.md`:
- Around line 89-107: Update T005’s acceptance criteria and completion note to
remove the requirement to test or reject republishing an existing tag, since
that behavior belongs to T012 in release.yml. Keep T005 focused on Cargo.toml
version/tag validation and previous_release_tag selection, and retain its
completed status only for that scope.
---
Nitpick comments:
In @.github/scripts/tests/test-previous-release-tag.sh:
- Around line 48-69: Extend the fixture around the existing tag setup by adding
a non-release-format tag such as v1.1.0-rc1 between v1.1.0 and v2.0.0, then
retain or update the v2.0.0 assertion to verify it still returns v1.1.0 rather
than the prerelease tag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 46566e32-8f12-41fa-a8ab-3ed028d696a5
📒 Files selected for processing (5)
.github/scripts/previous-release-tag.sh.github/scripts/tests/test-previous-release-tag.sh.github/scripts/tests/test-validate-release-tag.sh.github/scripts/validate-release-tag.shspecs/005-ci-release-versioning/tasks.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…s, header - previous-release-tag.sh: git describe's --match "v[0-9]*" is a glob, not a regex -- it also matches non-release tags like v1.1.0-rc1. Walk back past any non-strict-SemVer match instead of returning the first hit. Verified the bug empirically before fixing (a throwaway repo with v1.0.0/v1.1.0/v1.1.0-rc1/v2.0.0 returned v1.1.0-rc1 as v2.0.0's "previous release" before this fix); added the same fixture as a permanent regression test. - validate-release-tag.sh: awk pattern required version/[workspace.package] to start at column 0, which TOML doesn't require; allow leading whitespace. Also removed the header's inaccurate FR-011 claim -- that logic lives in previous-release-tag.sh, not here. - test-validate-release-tag.sh: replaced predictable/racy /tmp/out.$$ capture files with mktemp under the test's own private tmpdir.
|
Re: PR-level review comments (no thread to reply on directly): sourcery (review 4819125782) — both points confirmed valid, fixed in 239f119:
coderabbitai (review 4819214130, prompt to extend the fixture) — implemented exactly as suggested: added greptile — hit its trial credit limit again, no actual review content; nothing to action. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/validate-release-tag.sh (1)
22-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnforce strict SemVer numeric identifiers
Both regexes still accept
v01.2.3-style tags; replace each[0-9]+with0|[1-9][0-9]*in.github/scripts/validate-release-tag.shand.github/scripts/previous-release-tag.sh.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/validate-release-tag.sh around lines 22 - 25, Update the release-tag regex in validate-release-tag.sh at lines 22-25 and previous-release-tag.sh at lines 27-28, replacing each numeric [0-9]+ component with 0|[1-9][0-9]*. Preserve the vMAJOR.MINOR.PATCH structure while rejecting identifiers with leading zeroes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/scripts/validate-release-tag.sh:
- Around line 22-25: Update the release-tag regex in validate-release-tag.sh at
lines 22-25 and previous-release-tag.sh at lines 27-28, replacing each numeric
[0-9]+ component with 0|[1-9][0-9]*. Preserve the vMAJOR.MINOR.PATCH structure
while rejecting identifiers with leading zeroes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 424dbb11-74c6-4cda-9f23-2a849626b022
📒 Files selected for processing (4)
.github/scripts/previous-release-tag.sh.github/scripts/tests/test-previous-release-tag.sh.github/scripts/tests/test-validate-release-tag.sh.github/scripts/validate-release-tag.sh
|
While independently implementing T005 in a separate session (before spotting this branch already existed), I noticed a gap worth flagging here rather than opening a competing PR. Missing: duplicate-release rejection. T005's acceptance criteria (and the spec's edge cases) call for rejecting re-publishing an existing tag. Neither Worth noting why a local git-tag-existence check won't work here: by the time In my scratch version I made that check injectable via an env var (default 🧙 Noted via WOZCODE |
|
Follow-up on my own comment above: no code change needed here after all.
So this branch already has the right design — same reasoning I gave above (git-ref presence can't distinguish "just-pushed tag" from "already-published release"), just correctly deferred to the task that actually owns |
# Conflicts: # specs/005-ci-release-versioning/tasks.md
2 participants
Summary
.github/scripts/validate-release-tag.sh: validates a release tag againstCargo.toml's[workspace.package].version(FR-010), fails naming both values on mismatch (SC-006)..github/scripts/previous-release-tag.sh: resolves the nearest prior SemVer tag (FR-011), empty output (not an error) for the first-release case (FR-009).tasks.md.Despite the branch name (leftover from initial scoping), this PR is T005 only — T006 (build-identifier) is a separate, upcoming PR to keep one concern per PR.
Why
First real Phase 2 "Foundational" task for epic 005 — these scripts are what
release.yml(T008, not yet built) will call.Scope note
T005's own description mentions asserting "re-publishing an existing tag is rejected" — that check isn't in these scripts. It requires querying the GitHub Release API (does a release already exist for this tag?), which these are deliberately offline/git-only and can't determine. That check is T012's job in
release.yml, where it's already documented.Test plan
bash .github/scripts/tests/test-validate-release-tag.sh— 8/8 passbash .github/scripts/tests/test-previous-release-tag.sh— 4/4 passCargo.toml(v0.1.0passes,v9.9.9fails with both values named)cargo build --workspaceunaffected (no Rust files touched)🧙 Built with WOZCODE
Summary by Sourcery
Add release tag validation and previous-tag resolution scripts backed by fixture-based tests, and mark CI release versioning task T005 as complete.
New Features:
Documentation:
Tests:
Summary by cubic
Adds two Bash scripts to validate a release tag against
[workspace.package].versionand to resolve the previous SemVer tag, with fixture tests. Completes epic 005 T005 and prepares inputs forrelease.yml.New Features
.github/scripts/validate-release-tag.sh: EnforcesvMAJOR.MINOR.PATCHand exact match toCargo.toml[workspace.package].version; on mismatch, error names both values..github/scripts/previous-release-tag.sh: Finds the nearest previous strict SemVerv*tag; prints nothing for the first release. Re-publish check is deferred to T012 inrelease.yml.Bug Fixes
previous-release-tag.sh: Skips prerelease tags matched bygit describe --match "v[0-9]*"by walking back to the nearest strictvMAJOR.MINOR.PATCH; regression test added.validate-release-tag.sh: Accepts leading whitespace in TOML when reading[workspace.package].version; tests now usemktempfor safe temp files.Written for commit 239f119. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Documentation