Add semantic versioning for releases - #471
Conversation
This comment has been minimized.
This comment has been minimized.
35e7193 to
30ad08d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Generate vX.Y.Z, vX.Y, and vX tags for release images - Validate semver format (vX.Y.Z or vX.Y.Z-prerelease) - Skip latest tag for pre-release versions (-alpha, -beta, -rc) Addresses tasks 1-1, 1-2, 1-3 for issue #389
- Validates semver format (vX.Y.Z or vX.Y.Z-prerelease) - Creates version tag and floating tags (vX.Y, vX) - Supports --dry-run mode for testing - Generates markdown release notes template Addresses tasks 2-1, 2-2, 2-3 for issue #389
Pin all action references to the v0 floating tag for stability: - sdlc-pipeline.yml (5 uses + 1 input default) - reusable-review.yml - reusable-autofix.yml - reusable-conflict-resolve.yml - on-review-feedback.yml - on-mention.yml - on-push-doc-updater.yml - self-improvement.yml Addresses tasks 3-1 through 3-8 for issue #389
- Add versioning policy section to README.md - Update action example to use @v0 in README.md - Add version pinning section to reusable-workflows.md - Update all workflow examples to use @v0 - Add versioned reference example to action/README.md - Create RELEASING.md with release process guide Addresses tasks 4-1 through 4-4 for issue #389
30ad08d to
74f0c90
Compare
|
Resolved merge conflicts via rebase onto Conflicts resolved:
All changes verified locally (ruff check passed, YAML valid). — Authored by egg |
This comment has been minimized.
This comment has been minimized.
|
@james-in-a-box pull main into this branch |
|
The branch is already up to date with Verified that commit — Authored by egg |
This comment has been minimized.
This comment has been minimized.
|
@james-in-a-box checks are failing on this pr due to the labels missing. Let's create a follow up issue for updating versions for GitHub actions and do that after this is merged so we can get checks passing on this branch. |
|
Created follow-up issue #475 for updating action references to The CI failures were caused by a chicken-and-egg problem: the workflows reference Changes made:
Files modified:
The documentation still references — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Agent-Mode Design Review: No concerns.
This PR implements semantic versioning infrastructure for Docker images and GitHub Action references. All changes are standard CI/CD automation:
- Release workflow (
release-images.yml): Generates version tags via shell logic — no agent interaction - Release script (
create-release.sh): Human-run tooling for creating releases - Action reference updates: Mechanical
@main→@v0replacements across workflows - Documentation: Explains versioning policy for human consumers
None of these changes involve agent prompts, agent output handling, or agent workflow design. The PR is purely about versioning infrastructure for the project itself.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review Summary
This PR implements semantic versioning infrastructure for Docker images and GitHub Action references. The implementation is generally sound, but I found several issues that should be addressed.
Issues Requiring Changes
1. Workflow Logic Bug: Push to Main Now Produces Incorrect Tags
File: .github/workflows/release-images.yml (lines 110-113)
Current logic:
elif [[ "$TAG" != "latest" ]]; then
# Non-release push to main: add latest tag
TAGS="${TAGS},${IMAGE}:latest"
fiProblem: When github.event_name == "push" (push to main), the code sets TAG="latest" on line 63. Then this elif condition is false (since TAG == "latest"), so no additional tags are added. The final TAGS will be just ${IMAGE}:latest.
This matches the original behavior, but the comment is misleading. More importantly, the logic is fragile—if someone changes line 63 to set a different default tag (e.g., main), this condition would suddenly add :latest as a second tag, which may not be intended.
Suggestion: Simplify by making the push-to-main case explicit:
if [[ "$IS_RELEASE" == "true" ]]; then
# release logic...
else
# Non-release: just use the tag (already set to "latest" for push events)
# No additional tags needed
:
fiOr document the current behavior more clearly with a comment explaining that push-to-main sets TAG="latest" so this elif is effectively a no-op.
2. Pre-release Tags Still Update Floating Tags (vX, vX.Y)
File: .github/workflows/release-images.yml (lines 102-109)
Problem: For pre-releases like v1.0.0-alpha, the code adds floating tags v1.0 and v1:
if [[ "$IS_RELEASE" == "true" ]]; then
TAGS="${TAGS},${IMAGE}:${MINOR_TAG},${IMAGE}:${MAJOR_TAG}"
# Only skips :latest for pre-releasesThis means v1.0.0-alpha will update the v1 and v1.0 floating tags, which is typically not the desired behavior. Users pinned to @v1 expect stable releases only.
The create-release.sh script has the same issue (lines 179-183)—it always updates floating tags, even for pre-releases.
Suggestion: Skip floating tag updates for pre-releases:
if [[ "$IS_RELEASE" == "true" ]]; then
if [[ "$IS_PRERELEASE" == "false" ]]; then
TAGS="${TAGS},${IMAGE}:${MINOR_TAG},${IMAGE}:${MAJOR_TAG}"
TAGS="${TAGS},${IMAGE}:latest"
fi
# Pre-releases only get the exact version tag (e.g., v1.0.0-alpha)
fiAnd update create-release.sh similarly:
if [[ "$IS_PRERELEASE" == "false" ]]; then
run_cmd git tag -f "$MINOR_TAG" ...
run_cmd git tag -f "$MAJOR_TAG" ...
fi3. Create-Release Script: PATCH Variable Declared but Never Used
File: .github/scripts/create-release.sh (line 120)
PATCH="${BASH_REMATCH[3]}"This variable is extracted but never used. Either remove it or use it (perhaps in the release notes template).
4. Documentation Inconsistency with Implementation
Files: README.md, action/README.md, docs/guides/reusable-workflows.md
The documentation shows examples using @v0, but:
- The
@v0tag doesn't exist yet (as noted in commit message and issue #475) - The actual workflow files reference
@main
While issue #475 is meant to address this, the current state is confusing. The documentation promises something that won't work until after the first release.
Suggestion: Either:
- Add a note in the docs: "Note: Use
@mainuntil the first release (v0.1.0) is published, which will create the@v0tag." - Or wait to merge the documentation changes until after the first release
5. Create-Release Script: Missing Sanity Check for Branch
File: .github/scripts/create-release.sh
The script checks for uncommitted changes but doesn't verify it's running from an appropriate branch (e.g., main). Running create-release.sh v1.0.0 from a feature branch would create a release pointing to that branch's HEAD.
Suggestion: Add a warning or check:
CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
if [[ "$CURRENT_BRANCH" != "main" ]]; then
log_warn "Creating release from branch '$CURRENT_BRANCH' (not main)"
read -p "Continue? [y/N] " -n 1 -r
echo
if [[ ! $REPLY =~ ^[Yy]$ ]]; then
exit 1
fi
fi6. Workflow: BASH_REMATCH Used After Separate Regex Check
File: .github/workflows/release-images.yml (lines 59, 68, 73-75)
The code first checks $TAG =~ ^v[0-9]+\.[0-9]+\.[0-9]+ (line 59), but the capture groups are used from a different regex on line 68. This works because the second regex is run when IS_RELEASE == "true", but it's fragile—if someone modifies line 59's regex to also capture groups, they might expect those to be used.
The current code is correct, but consider consolidating to make the flow clearer.
Minor Issues / Suggestions
7. RELEASING.md Rollback Example Contains a Subtle Issue
File: RELEASING.md (lines 104-108)
# Delete the bad tags
git push --delete origin v0.2.0 v0.2 v0
git tag -d v0.2.0 v0.2 v0
# Point floating tags to the last good release
git tag -f v0.2 v0.1.5If you're releasing v0.2.0 and it's bad, v0.1.5 doesn't exist—you'd want v0.1.x (whatever the last v0.1 release was). The example is illustrative but may confuse readers.
8. Script Output May Contain Untrusted Data
File: .github/scripts/create-release.sh (line 206 onwards)
The heredoc for release notes includes $PREV_TAG which comes from git describe. If tag names contained special characters, they could break the heredoc or the markdown. This is low risk since tag names are controlled, but worth noting.
What Looks Good
- The semver validation regex is correct and handles pre-release suffixes
- The dry-run mode in
create-release.shis well implemented - The workflow structure follows existing patterns
- Documentation is comprehensive
- The chicken-and-egg problem (needing v0 to exist before referencing it) is properly handled via issue #475
Recommendation
Request changes due to issues #1-2 (logic bugs affecting release behavior). The floating tag update for pre-releases (#2) is particularly important to fix before this ships—it would cause pre-release versions to override the stable floating tags.
— Authored by egg
There was a problem hiding this comment.
Contract Verification Review
Summary
This PR implements semantic versioning infrastructure for Docker images and GitHub Action references. The implementation is partially complete - core infrastructure (Phases 1, 2, 4) is implemented correctly, but Phase 3 (action reference updates to @v0) was intentionally reverted due to a chicken-and-egg problem with the @v0 tag not existing yet.
Verified Criteria ✅
| Criterion | Task | Status |
|---|---|---|
| AC-1 | [TASK-1-1] On release v0.1.0, images are tagged as v0.1.0, v0.1, and v0 | ✅ Verified |
| AC-2 | [TASK-1-2] Workflow fails gracefully if tag doesn't match vX.Y.Z pattern | ✅ Verified |
| AC-3 | [TASK-1-3] Tags with -alpha, -beta, -rc suffix don't update latest | ✅ Verified |
| AC-4 | [TASK-2-1] Script validates version format, creates git tag, updates floating tags (vX.Y, vX), and pushes to origin | ✅ Verified |
| AC-5 | [TASK-2-2] Dry-run shows what would happen without making changes | ✅ Verified |
| AC-6 | [TASK-2-3] Script outputs a markdown template suitable for GitHub release body | ✅ Verified |
| AC-15 | [TASK-4-1] Documents semver policy, how to pin versions, and what breaking changes mean | ✅ Verified |
| AC-16 | [TASK-4-2] Examples updated to use @v0, explanation of version pinning added | ✅ Verified |
| AC-17 | [TASK-4-3] Quick start example uses versioned reference | ✅ Verified |
| AC-18 | [TASK-4-4] Step-by-step guide for creating releases, including checklist | ✅ Verified |
Not Verified (Intentionally Deferred) ⚠️
| Criterion | Task | Status | Reason |
|---|---|---|---|
| AC-7 | [TASK-3-1] sdlc-pipeline.yml references updated to @v0 | Reverted in 363d839 - v0 tag doesn't exist yet | |
| AC-8 | [TASK-3-2] reusable-review.yml updated | See #475 | |
| AC-9 | [TASK-3-3] reusable-autofix.yml updated | See #475 | |
| AC-10 | [TASK-3-4] reusable-conflict-resolve.yml updated | See #475 | |
| AC-11 | [TASK-3-5] on-review-feedback.yml updated | See #475 | |
| AC-12 | [TASK-3-6] on-mention.yml updated | See #475 | |
| AC-13 | [TASK-3-7] on-push-doc-updater.yml updated | See #475 | |
| AC-14 | [TASK-3-8] self-improvement.yml updated | See #475 |
Not Applicable (Post-Merge Activities) ℹ️
| Criterion | Task | Status | Reason |
|---|---|---|---|
| AC-19 | [TASK-5-1] CI green on main branch | ℹ️ N/A | Post-merge verification |
| AC-20 | [TASK-5-2] Git tags exist, GitHub release created, images pushed | ℹ️ N/A | Post-merge activity |
| AC-21 | [TASK-5-3] docker pull ghcr.io/jwbron/egg-sandbox:v0.1.0 succeeds | ℹ️ N/A | Post-release verification |
Verification Details
Phase 1 (release-images.yml):
.github/workflows/release-images.yml:102-104: Correctly generatesvX.Y.Z,vX.Y,vXtags.github/workflows/release-images.yml:67-71: Validates semver with proper error annotation.github/workflows/release-images.yml:80-83,106-109: Correctly skipslatestfor prereleases
Phase 2 (create-release.sh):
.github/scripts/create-release.sh:100-106: Validates semver format.github/scripts/create-release.sh:165-178: Creates tags and pushes to origin.github/scripts/create-release.sh:61-67: Implementsrun_cmd()for dry-run mode.github/scripts/create-release.sh:187-259: Generates markdown release notes template
Phase 4 (Documentation):
README.md:266-300: Versioning section with policy and examplesdocs/guides/reusable-workflows.md:9-24: Version Pinning section addedaction/README.md:26-45: Quick Start uses @v0, Version Pinning section addedRELEASING.md: Complete release guide with checklist
Contract Compliance
The PR correctly addresses the chicken-and-egg problem by:
- Creating follow-up issue #475 to track the deferred @v0 reference updates
- Providing clear commit message explaining the reversion reason
- Keeping documentation examples at @v0 (they serve as future-state documentation)
This is a reasonable approach since workflows cannot reference a tag that doesn't exist yet.
Recommendation
Approve - The core infrastructure for semantic versioning is correctly implemented. Phase 3 tasks are appropriately deferred to a follow-up issue (#475) with clear rationale. The PR provides all necessary tooling to create the first release, after which #475 can be completed.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@james-in-a-box I had to revert this change due to the above feedback not being addressed. Open up a new PR with the changes in this branch so we can clean it up. |
This comment has been minimized.
This comment has been minimized.
|
@james-in-a-box I had to revert this change due to the above feedback not being addressed. Open up a new PR with the changes in this branch so we can clean it up. |
|
egg run failed (exit code: unknown). View run logs |
Re-applies the semantic versioning infrastructure from #471 (reverted in #479) with all feedback from the review addressed: - Fix pre-release floating tags: pre-releases (e.g., v1.0.0-alpha) no longer update vX/vX.Y floating tags or the latest Docker tag. Users pinned to @v0 now only receive stable releases. - Simplify push-to-main tagging: make the non-release case explicit with a clear comment instead of a fragile elif condition. - Remove unused PATCH variable from create-release.sh. - Add branch safety check: create-release.sh warns when run from a branch other than main and prompts for confirmation. - Consolidate BASH_REMATCH: the workflow now uses a single regex for both validation and capture, eliminating fragile two-regex flow. - Fix rollback example: RELEASING.md now uses realistic version numbers and explains the rollback target should be the actual last good release. - Sanitize git-describe output in release notes heredoc. - Add @main-vs-@v0 notes: docs explain to use @main until the first release creates the @v0 tag (addresses doc/implementation gap).
* Re-land semantic versioning with review fixes Re-applies the semantic versioning infrastructure from #471 (reverted in #479) with all feedback from the review addressed: - Fix pre-release floating tags: pre-releases (e.g., v1.0.0-alpha) no longer update vX/vX.Y floating tags or the latest Docker tag. Users pinned to @v0 now only receive stable releases. - Simplify push-to-main tagging: make the non-release case explicit with a clear comment instead of a fragile elif condition. - Remove unused PATCH variable from create-release.sh. - Add branch safety check: create-release.sh warns when run from a branch other than main and prompts for confirmation. - Consolidate BASH_REMATCH: the workflow now uses a single regex for both validation and capture, eliminating fragile two-regex flow. - Fix rollback example: RELEASING.md now uses realistic version numbers and explains the rollback target should be the actual last good release. - Sanitize git-describe output in release notes heredoc. - Add @main-vs-@v0 notes: docs explain to use @main until the first release creates the @v0 tag (addresses doc/implementation gap). * Fix PREV_TAG validation for consistent untrusted data handling Replace the partial sanitization approach (PREV_TAG_DISPLAY used only in comments) with upfront validation. Now PREV_TAG is validated against a whitelist of safe git ref characters before use. This ensures the same validated value is used in both the git log command and the output. The validation pattern allows alphanumeric, dots, underscores, hyphens, and forward slashes (for hierarchical refs like release/v1.0.0). Authored-by: egg --------- Co-authored-by: egg <egg@example.com> Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com>
Add semantic versioning for releases
Implements semantic versioning for Docker images and GitHub Action
references, enabling version pinning for stability and rollback.
with semver validation and pre-release support
guide, and new RELEASING.md
Closes #389
Test plan:
verify image tagging logic
.github/scripts/create-release.sh v0.0.0-test --dry-runtovalidate the release script
Authored-by: egg