feat(release): add build-identifier script (epic 005 T006) - #43
Conversation
Test-first per Constitution I. GITHUB_RUN_NUMBER.GITHUB_RUN_ATTEMPT, validated as present and base-10 numeric (not just concatenated blindly) so SC-003's "strictly increasing" check has a numeric pair to compare, not two arbitrary strings.
Reviewer's GuideImplements a deterministic build identifier script for GitHub Actions releases, along with fixture-based bash tests, and marks the corresponding task as completed in the CI/release specification. Sequence diagram for build-identifier.sh execution in GitHub ActionssequenceDiagram
actor Workflow
participant BuildIdentifierScript as build_identifier_sh
Workflow->>BuildIdentifierScript: execute build-identifier.sh
BuildIdentifierScript->>BuildIdentifierScript: read GITHUB_RUN_NUMBER
BuildIdentifierScript->>BuildIdentifierScript: read GITHUB_RUN_ATTEMPT
alt missing_env_vars
BuildIdentifierScript->>Workflow: stderr "error: GITHUB_RUN_NUMBER and GITHUB_RUN_ATTEMPT must both be set"
BuildIdentifierScript->>Workflow: exit 1
else non_numeric_env_vars
BuildIdentifierScript->>Workflow: stderr "error: ... must both be base-10 integers"
BuildIdentifierScript->>Workflow: exit 1
else valid_env_vars
BuildIdentifierScript->>Workflow: stdout run_number.run_attempt
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
The PR introduces the build-identifier script and its associated tests. Although the implementation aligns with the requirement to use GITHUB_RUN_NUMBER and GITHUB_RUN_ATTEMPT, the PR is currently not up to standards due to quality and reliability issues in the test suite. A significant logic error was found in the shell script tests where the 'env' command is misconfigured, causing tests to pass for the wrong reasons. Additionally, there are multiple linting violations for unused variables in the test script. These issues must be addressed to ensure the build process remains robust and maintainable.
Test suggestions
- Valid numeric environment variables result in a concatenated run_number.run_attempt output.\n- [x] Missing GITHUB_RUN_NUMBER or GITHUB_RUN_ATTEMPT environment variables causes the script to exit with an error.\n- [x] Non-numeric strings (e.g., 'abc') provided as environment variables cause the script to exit with an error.\n- [x] Negative numbers provided as environment variables cause the script to exit with an error.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
📝 WalkthroughWalkthroughThe PR adds a Bash script that validates ChangesBuild identifier generation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ppressing lint - The GITHUB_RUN_ATTEMPT-missing test had `env`'s `-u` option after a NAME=VALUE assignment. env stops parsing options at the first assignment, so it tried to exec a program literally named "-u" and failed with "No such file or directory" -- the test still passed, but for the wrong reason (env failing, not the script's own validation). Verified: reproduced the bug locally (env: -u: No such file or directory), confirmed the fix actually reaches the script's own "must both be set" error instead. - a_attempt/b_attempt were assigned but never used (ShellCheck noise). Rather than prefix-and-ignore, added a real second assertion that exercises them: same run-number, attempt breaks the tie -- a more useful test than a suppressed warning.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@specs/005-ci-release-versioning/tasks.md`:
- Around line 99-105: Add an ADR under specs/decisions/ documenting the
GITHUB_RUN_NUMBER.GITHUB_RUN_ATTEMPT build-identifier contract, including its
numeric ordering semantics, release metadata usage, and artifact naming. Update
T006 in tasks.md to reference the new ADR before merging, while preserving its
existing completion details.
🪄 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: e4ab5f21-7c38-4860-be25-488a6a286fac
📒 Files selected for processing (3)
.github/scripts/build-identifier.sh.github/scripts/tests/test-build-identifier.shspecs/005-ci-release-versioning/tasks.md
# Conflicts: # specs/005-ci-release-versioning/tasks.md
Summary
.github/scripts/build-identifier.sh: computesGITHUB_RUN_NUMBER.GITHUB_RUN_ATTEMPT(FR-005), validating both are present and base-10 numeric rather than blindly concatenating.tasks.md.Separate PR from #42 (T005) — different scripts, no shared code, one concern each.
Why
Second Phase 2 "Foundational" task —
release.yml(T008, not yet built) and the release-notes script (T014) will both consume this identifier.Test plan
bash .github/scripts/tests/test-build-identifier.sh— 8/8 pass (happy path, missing/non-numeric env vars, negative numbers).cargo build --workspaceunaffected (no Rust files touched).🧙 Built with WOZCODE
Summary by Sourcery
Add a script to compute a deterministic build identifier for CI release runs and mark the corresponding task as completed.
New Features:
Documentation:
Tests:
Summary by cubic
Adds a script to compute a deterministic build ID
GITHUB_RUN_NUMBER.GITHUB_RUN_ATTEMPTwith strict numeric validation to support numeric release ordering (FR-005/SC-003). Updates tests and marks epic 005 T006 done; tasks doc synced with main.New Features
.github/scripts/build-identifier.shoutputsrun_number.attemptand fails if either env var is missing or non-numeric.specs/005-ci-release-versioning/tasks.mdmarks T006 done.Bug Fixes
GITHUB_RUN_ATTEMPTtest by placingenv -ubefore assignments so the script’s validation runs.Written for commit deda4ba. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Documentation