Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 91 additions & 30 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
name: Release

on:
pull_request:
types: [closed]
push:
branches: [main]
workflow_dispatch:
inputs:
Expand All @@ -18,18 +17,91 @@ on:
permissions:
contents: read

concurrency:
group: release
cancel-in-progress: false

jobs:
resolve-release-context:
name: Resolve release context
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
outputs:
should-release: ${{ steps.resolve.outputs.should-release }}
bump-type: ${{ steps.resolve.outputs.bump-type }}
steps:
- name: Resolve release request
id: resolve
env:
GH_TOKEN: ${{ github.token }}
EVENT_NAME: ${{ github.event_name }}
CURRENT_REF: ${{ github.ref }}
DEFAULT_BRANCH: main
PUSH_SHA: ${{ github.sha }}
PUSH_HEAD_COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
INPUT_BUMP_TYPE: ${{ inputs.bump_type }}
run: |
set -euo pipefail

SHOULD_RELEASE=false
BUMP_TYPE=""

if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
if [ "$CURRENT_REF" != "refs/heads/$DEFAULT_BRANCH" ]; then
echo "::notice::Skipping release because workflow_dispatch must be run on $DEFAULT_BRANCH, got $CURRENT_REF."
else
BUMP_TYPE="$INPUT_BUMP_TYPE"
if [ -n "$BUMP_TYPE" ]; then
SHOULD_RELEASE=true
echo "✓ Manual release requested on $DEFAULT_BRANCH with bump type $BUMP_TYPE"
else
echo "::notice::Skipping manual release because no bump type was provided."
fi
fi

echo "should-release=$SHOULD_RELEASE" >> "$GITHUB_OUTPUT"
echo "bump-type=$BUMP_TYPE" >> "$GITHUB_OUTPUT"
exit 0
fi

if [[ "${PUSH_HEAD_COMMIT_MESSAGE:-}" == *"[version bump]"* || "${PUSH_HEAD_COMMIT_MESSAGE:-}" == "Bump version to "* || "${PUSH_HEAD_COMMIT_MESSAGE:-}" == "chore: Release v"* || "${PUSH_HEAD_COMMIT_MESSAGE:-}" == "chore: release "* ]]; then
echo "::notice::Skipping release for the automated version bump commit."
echo "should-release=false" >> "$GITHUB_OUTPUT"
echo "bump-type=" >> "$GITHUB_OUTPUT"
exit 0
fi

PRS_JSON=$(gh api "repos/${{ github.repository }}/commits/$PUSH_SHA/pulls")
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | index("release")))] | first // {}')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 jq index() returns 0 for the first label, which is falsy in jq

index("release") returns the numeric position of the element. In jq, 0 is falsy, so select(...and (.labels | map(.name) | index("release"))) silently drops PRs where "release" is the very first label — a realistic ordering when it's the first label added. The release would be quietly skipped with no bump type found.

The bump-label checks on lines 75–80 are safe because they use != null, but this initial filter is not. Replace the truthy test with an explicit presence check:

Suggested change
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | index("release")))] | first // {}')
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | contains(["release"])))] | first // {}')
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 73

Comment:
**`jq index()` returns 0 for the first label, which is falsy in jq**

`index("release")` returns the numeric position of the element. In jq, `0` is falsy, so `select(...and (.labels | map(.name) | index("release")))` silently drops PRs where `"release"` is the very first label — a realistic ordering when it's the first label added. The release would be quietly skipped with no bump type found.

The bump-label checks on lines 75–80 are safe because they use `!= null`, but this initial filter is not. Replace the truthy test with an explicit presence check:

```suggestion
MATCHING_PR_JSON=$(echo "$PRS_JSON" | jq -c --arg branch "$DEFAULT_BRANCH" '[.[] | select(.merged_at != null and .base.ref == $branch and (.labels | map(.name) | contains(["release"])))] | first // {}')
```

How can I resolve this? If you propose a fix, please make it concise.

if [ "$MATCHING_PR_JSON" != "{}" ]; then
if echo "$MATCHING_PR_JSON" | jq -e '(.labels | map(.name) | index("bump-major")) != null' >/dev/null; then
BUMP_TYPE=major
elif echo "$MATCHING_PR_JSON" | jq -e '(.labels | map(.name) | index("bump-minor")) != null' >/dev/null; then
BUMP_TYPE=minor
elif echo "$MATCHING_PR_JSON" | jq -e '(.labels | map(.name) | index("bump-patch")) != null' >/dev/null; then
BUMP_TYPE=patch
fi
fi

if [ -z "$BUMP_TYPE" ]; then
ASSOCIATED_PRS=$(echo "$PRS_JSON" | jq -r '[.[].number | tostring] | join(", ")')
if [ -n "$ASSOCIATED_PRS" ]; then
echo "::notice::Skipping release because push $PUSH_SHA is associated with PR(s) [$ASSOCIATED_PRS], but none currently have the release label with a bump label."
else
echo "::notice::Skipping release because push $PUSH_SHA is not associated with a merged PR to $DEFAULT_BRANCH that has the release label and a bump label."
fi
echo "should-release=false" >> "$GITHUB_OUTPUT"
echo "bump-type=" >> "$GITHUB_OUTPUT"
exit 0
fi

RELEASE_PR=$(echo "$MATCHING_PR_JSON" | jq -r '"#\(.number)"')
echo "✓ Release requested by push $PUSH_SHA via $RELEASE_PR with bump type $BUMP_TYPE"
echo "should-release=true" >> "$GITHUB_OUTPUT"
echo "bump-type=$BUMP_TYPE" >> "$GITHUB_OUTPUT"

check-release-label:
name: Check for release label
if: |
github.event_name == 'workflow_dispatch' ||
(github.event_name == 'pull_request' &&
github.event.pull_request.merged == true &&
contains(github.event.pull_request.labels.*.name, 'release'))
needs: resolve-release-context
if: needs.resolve-release-context.outputs.should-release == 'true'
runs-on: ubuntu-latest
outputs:
should-release: ${{ steps.check.outputs.should-release }}
Expand All @@ -38,29 +110,15 @@ jobs:
- name: Check release conditions
id: check
env:
EVENT_NAME: ${{ github.event_name }}
BUMP_TYPE: ${{ inputs.bump_type }}
HAS_BUMP_MAJOR: ${{ contains(github.event.pull_request.labels.*.name, 'bump-major') }}
HAS_BUMP_MINOR: ${{ contains(github.event.pull_request.labels.*.name, 'bump-minor') }}
HAS_BUMP_PATCH: ${{ contains(github.event.pull_request.labels.*.name, 'bump-patch') }}
BUMP_TYPE: ${{ needs.resolve-release-context.outputs.bump-type }}
run: |
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
if [ -n "$BUMP_TYPE" ]; then
echo "bump-type=$BUMP_TYPE" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
echo "Manual release requested with bump type '$BUMP_TYPE'"
elif [ "$HAS_BUMP_MAJOR" = "true" ]; then
echo "bump-type=major" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
elif [ "$HAS_BUMP_MINOR" = "true" ]; then
echo "bump-type=minor" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
elif [ "$HAS_BUMP_PATCH" = "true" ]; then
echo "bump-type=patch" >> "$GITHUB_OUTPUT"
echo "should-release=true" >> "$GITHUB_OUTPUT"
echo "Release requested with bump type $BUMP_TYPE"
else
echo "should-release=false" >> "$GITHUB_OUTPUT"
fi
Comment on lines 101 to 121
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 check-release-label is now a passthrough with no logic of its own

After this PR, the job only re-emits the bump-type and should-release values already set by resolve-release-context — its name no longer matches its behaviour and the "Has no superfluous parts" simplicity rule is violated. The only real purpose it still serves is acting as a dependency gate for notify-approval-needed.

Consider either removing it and wiring notify-approval-needed directly off resolve-release-context, or renaming it to something like gate-release to accurately describe its current role.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 101-121

Comment:
**`check-release-label` is now a passthrough with no logic of its own**

After this PR, the job only re-emits the `bump-type` and `should-release` values already set by `resolve-release-context` — its name no longer matches its behaviour and the "Has no superfluous parts" simplicity rule is violated. The only real purpose it still serves is acting as a dependency gate for `notify-approval-needed`.

Consider either removing it and wiring `notify-approval-needed` directly off `resolve-release-context`, or renaming it to something like `gate-release` to accurately describe its current role.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


notify-approval-needed:
name: Notify Slack - Approval Needed
needs: check-release-label
Expand All @@ -75,11 +133,14 @@ jobs:

release:
name: Bump version and release
needs: [check-release-label, notify-approval-needed]
needs: [resolve-release-context, check-release-label, notify-approval-needed]
runs-on: ubuntu-latest
concurrency:
group: release
cancel-in-progress: false
# Use `always()` so the release proceeds even if the Slack notification fails —
# a Slack outage shouldn't block releases.
if: always() && needs.check-release-label.outputs.should-release == 'true'
if: always() && needs.resolve-release-context.outputs.should-release == 'true' && needs.check-release-label.outputs.should-release == 'true'
environment: Release
permissions:
contents: write
Expand Down
Loading