Skip to content
Open
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
129 changes: 124 additions & 5 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,147 @@ on:
- patch
- minor
- major
schedule:
# Weekly release, Monday 09:17 UTC. Scheduled runs fail closed if CI for
# the exact release SHA is missing or incomplete, or if the prior release
# did not reach both GitHub Releases and PyPI. A post-tag publishing failure
# requires maintainer recovery of that exact version; a later schedule will
# not skip over it.
- cron: '17 9 * * 1'

permissions:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Add a concurrency group and a repository guard

There is no concurrency: anywhere in this file (I grepped the whole thing at 070f7ec). Two runs of this job can overlap the moment a cron exists — most obviously a maintainer dispatching a release around Monday 09:00.

I traced what actually happens and the good news is it does not double-release: both runs check out the same SHA, both compute 2.4.2, both commit and tag locally, the loser's git push is rejected non-fast-forward, and because line 75 is git push && git push --tags the && short-circuits so no stray tag is pushed and the job dies before Create GitHub Release. But it is a guaranteed red run plus a confusing half-state to reason about at exactly the moment someone is trying to ship. Serialize instead:

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

Separately, on the release job:

if: github.repository == 'awslabs/cli-agent-orchestrator'

GitHub disables scheduled workflows in forks by default, so this is belt-and-braces — but note the failure mode if a fork does enable Actions: secrets.RELEASE_DEPLOY_KEY resolves empty, actions/checkout silently falls back to token auth, and the job holds contents: write, so the push succeeds and the fork tags and Releases its own main every Monday. One line closes it.

contents: read

# Never cancel an in-flight release; queue manual and scheduled releases.
concurrency:
group: release
cancel-in-progress: false

jobs:
preflight:
if: github.repository == 'awslabs/cli-agent-orchestrator'
runs-on: ubuntu-latest
permissions:
actions: read
contents: read
outputs:
bump: ${{ steps.policy.outputs.bump }}
count: ${{ steps.policy.outputs.count }}
last_tag: ${{ steps.policy.outputs.last_tag }}
should_release: ${{ steps.policy.outputs.should_release }}
steps:
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
with:
fetch-depth: 0

- name: Evaluate scheduled release policy
id: policy
env:
GH_TOKEN: ${{ github.token }}
run: |
set -euo pipefail

if [ "$GITHUB_EVENT_NAME" != "schedule" ]; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] workflow_dispatch bypasses every gate this PR just added

The early return sends all non-schedule events straight to should_release=true, skipping the CI-success check, the prior-release completeness check, and the tag resolution. So the manual path can still cut a release from a commit whose CI is red or still running.

That matters here specifically because main has no safety net of its own — I re-confirmed just now:

GET /repos/awslabs/cli-agent-orchestrator/branches/main/protection
-> 404 "Branch not protected"

With no required status checks on main and no CI gate on manual releases, "CI was green" is enforced on exactly one of the two paths that can publish.

The checks are already written and only need LAST_TAG, so the cheapest fix is to keep the early return for bump selection only and let the CI + prior-release checks run for both events:

if [ "$GITHUB_EVENT_NAME" != "schedule" ]; then
  echo "bump=patch" >> "$GITHUB_OUTPUT"   # inputs.bump wins downstream anyway
  echo "should_release=true" >> "$GITHUB_OUTPUT"
fi
# ... then run tag resolution + CI/prior-release gates unconditionally,
# skipping only the "no new commits" early exit for manual runs.

echo "bump=patch" >> "$GITHUB_OUTPUT"
echo "count=manual" >> "$GITHUB_OUTPUT"
echo "last_tag=manual" >> "$GITHUB_OUTPUT"
echo "should_release=true" >> "$GITHUB_OUTPUT"
exit 0
fi

LAST_TAG=$(
git tag --merged "$GITHUB_SHA" --list 'v[0-9]*' --sort=-v:refname |
grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' |
head -n 1
)
if [ -z "$LAST_TAG" ]; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Scanning %b will misclassify a squash-merge body, and this branch is unreachable

Two separate things at this spot.

The -z branch never runs. LAST_TAG=$(git tag ... | grep -E ... | head -n 1) under set -euo pipefail: when grep matches nothing it exits 1, pipefail propagates that, and set -e kills the step before line 67. Verified:

$ bash -c 'set -euo pipefail; LAST_TAG=$(printf "abc\n" | grep -E "^v[0-9]" | head -n 1);
           if [ -z "$LAST_TAG" ]; then echo "friendly message printed"; exit 1; fi'
step exit code = 1        # message never printed

It still fails closed, so this is only about losing the diagnostic. LAST_TAG=$(... || true) restores it.

The body scan is the fragile part. git log --format='%s%n%b' at :123 feeds commit bodies into the classifier, and grep -E '^...' anchors to any line. This repo squash-merges, so bodies routinely carry full PR descriptions — and a body that quotes a changelog, pastes a nested commit list, or shows a fenced example containing feat:, fix!: or BREAKING CHANGE: silently escalates the bump. Combined with the P1 above, a stray quoted line is enough to cut a major.

Today you're lucky: in v2.4.1..main there is exactly one match and it is legitimate (subject-only matches = 0, subject+body = 1). That's a coincidence of content, not a property of the parser.

A ! marker is only meaningful in the subject, and BREAKING CHANGE: is only meaningful as a real footer, so scope them accordingly:

SUBJECTS=$(git log "${LAST_TAG}..${GITHUB_SHA}" --format='%s')
# footers only: last paragraph of each body
FOOTERS=$(git log "${LAST_TAG}..${GITHUB_SHA}" --format='%b' | grep -E '^BREAKING[ -]CHANGE:' || true)
if grep -Eq '^[a-z]+(\([^)]*\))?!:' <<<"$SUBJECTS" || [ -n "$FOOTERS" ]; then ...

Better still, the release job already installs git-cliff, which parses conventional commits properly and is driven by the cliff.toml this repo already ships — worth checking whether its bump support can replace the hand-rolled grep entirely, so the changelog and the version number can never disagree.

echo "No stable v<major>.<minor>.<patch> tag is reachable from $GITHUB_SHA"
exit 1
fi

COUNT=$(git rev-list "${LAST_TAG}..${GITHUB_SHA}" --count)
echo "count=$COUNT" >> "$GITHUB_OUTPUT"
echo "last_tag=$LAST_TAG" >> "$GITHUB_OUTPUT"
if [ "$COUNT" -eq 0 ]; then
echo "No commits since $LAST_TAG; skipping this scheduled release."
echo "bump=patch" >> "$GITHUB_OUTPUT"
echo "should_release=false" >> "$GITHUB_OUTPUT"
exit 0
fi

gh release view "$LAST_TAG" --repo "$GITHUB_REPOSITORY" >/dev/null
VERSION=${LAST_TAG#v}
python - "$VERSION" <<'PY'
import json
import sys
import urllib.error
import urllib.request

version = sys.argv[1]
url = f"https://pypi.org/pypi/cli-agent-orchestrator/{version}/json"
try:
with urllib.request.urlopen(url, timeout=30) as response:
json.load(response)
except (urllib.error.URLError, json.JSONDecodeError) as exc:
raise SystemExit(
f"Prior release v{version} is not verifiably published on PyPI: {exc}"
)
PY

# Maintained allow-list of release-blocking workflows. Add another
# workflow file here only when release policy requires it.
REQUIRED_WORKFLOWS=(ci.yml)
for workflow in "${REQUIRED_WORKFLOWS[@]}"; do
STATE=$(
gh run list \
--repo "$GITHUB_REPOSITORY" \
--commit "$GITHUB_SHA" \
--workflow "$workflow" \
--limit 10 \
--json conclusion,headSha,status \
--jq 'map(select(.headSha == env.GITHUB_SHA)) | first |
if . == null then "missing"
else "\(.status):\(.conclusion // "none")"
end'
)
if [ "$STATE" != "completed:success" ]; then
echo "$workflow is '$STATE' at $GITHUB_SHA; refusing to release"
exit 1
fi
done

COMMITS=$(git log "${LAST_TAG}..${GITHUB_SHA}" --format='%s%n%b')
if grep -Eq '(^[a-z]+(\([^)]*\))?!:|^BREAKING[ -]CHANGE:)' <<<"$COMMITS"; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] The next scheduled run cuts v3.0.0, unattended

I ran this exact algorithm against the current repo state:

LAST_TAG resolved = v2.4.1
commits since tag = 36
### bump the workflow would pick
BUMP=major

The trigger is genuine, not a parsing bug — commit e2e6318 (feat(workflow): async run submit, discovery, and live event following (#505) (#525)) carries a real Conventional Commits footer:

BREAKING CHANGE: `cao workflow run --json` emits `{run_id, state}` rather
than the full WorkflowRunResult, and the run-level `output` field is no
longer returned by GET /workflows/runs/{run_id}/result ...

So the classification is spec-correct. The problem is the blast radius, because nothing between the cron firing and a published major release involves a person. I traced it:

  1. release job has no environment: — I grepped the whole file, there is none — so contents: write at :141 commits the bump to main and pushes tag v3.0.0 with no approval.
  2. softprops/action-gh-release at :177 publishes GitHub Release v3.0.0.
  3. publish-to-pypi.yml triggers on release: types: [published], and its publish-testpypi job uses environment testpypi — which I checked live: {"protection_rules": []}. No approval. So cli-agent-orchestrator 3.0.0 lands on TestPyPI too.

Only the final publish-pypi job stops, because the pypi environment has required reviewers. By then the tag, the Release and the TestPyPI version number are all public and effectively unrecoverable — you cannot re-cut v3.0.0.

This also contradicts the PR title and #651, which both say patch.

I'd keep the semver derivation — that was the right fix — and just stop schedule from being able to do a major on its own:

          if grep -Eq '^feat(\([^)]*\))?:' <<<"$COMMITS"; then
            BUMP=minor
          else
            BUMP=patch
          fi
          # A major is never cut unattended: surface it and let a maintainer
          # run the workflow manually with bump=major.
          if grep -Eq '(^[a-z]+(\([^)]*\))?!:|^BREAKING[ -]CHANGE:)' <<<"$COMMITS"; then
            echo "::warning::Breaking changes since $LAST_TAG - run Release manually with bump=major"
            echo "should_release=false" >> "$GITHUB_OUTPUT"
            exit 0
          fi

Adding environment: with required reviewers to the release job would also work and is arguably better, since it covers the manual path too.

BUMP=major
elif grep -Eq '^feat(\([^)]*\))?:' <<<"$COMMITS"; then
BUMP=minor
else
BUMP=patch
fi

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] Require green CI on the release SHA before the bump

Confirming @call-me-ram's blocker, and the live configuration makes it worse than "a human was watching the badge." I queried the repo settings:

$ gh api repos/awslabs/cli-agent-orchestrator/branches/main/protection
{"message":"Branch not protected","status":"404"}

$ gh api repos/awslabs/cli-agent-orchestrator/environments --jq '.environments[]|{name,rules:[.protection_rules[].type]}'
{"name":"testpypi","rules":[]}
{"name":"pypi","rules":["branch_policy","required_reviewers"]}

So: main has no required status checks whatsoever, and the first environment the release pipeline touches — testpypi, publish-to-pypi.yml:185 — has no protection rules at all. Nothing in this repository currently guarantees the Monday-09:00 tip is green, and nothing stops a red tree from reaching TestPyPI unattended.

The ordering is what makes it unrecoverable rather than merely noisy. This job pushes the tag and creates the Release; publish-to-pypi.yml:4 fires on release: published; TestPyPI publishes with no gate; only then does publish-pypi (line 312) block on required_reviewers. By the time a maintainer is asked, the tag and the GitHub Release already exist, so the choice is between shipping a red build and leaving a permanent hole in the version sequence.

Add a schedule-only gate here, before Install git-cliff:

- name: Require green CI on the release SHA
  if: github.event_name == 'schedule' && steps.unreleased.outputs.count != '0'
  env:
    GH_TOKEN: ${{ github.token }}
  run: |
    # Deliberately keyed to $GITHUB_SHA, not the branch tip: checkout pinned
    # this run to github.sha, the unreleased count and the bump both ran on
    # that same tree, and the final push is fast-forward-only - so there is
    # no check/release race. Gating on "latest run on main" would reintroduce one.
    #
    # If Sunday-night CI is still in flight at 09:00 the conclusion is empty
    # and this fails, skipping the week. That is the intended failure mode:
    # an unverified release is worse than a late one.
    conclusion=$(gh run list --commit "$GITHUB_SHA" --workflow ci.yml \
      --json conclusion -q '.[0].conclusion')
    if [ "$conclusion" != "success" ]; then
      echo "CI is '${conclusion:-not started}' at $GITHUB_SHA - refusing to release"
      exit 1
    fi

Given main is unprotected, I'd treat this as required for merge, not a nice-to-have — without it the cron is the only path in this repository that can publish an artifact nobody and nothing has verified.

echo "bump=$BUMP" >> "$GITHUB_OUTPUT"
echo "should_release=true" >> "$GITHUB_OUTPUT"

release:
needs: preflight
if: >-
github.repository == 'awslabs/cli-agent-orchestrator' &&
needs.preflight.outputs.should_release == 'true'
runs-on: ubuntu-latest
permissions:
contents: write
steps:
- uses: actions/checkout@v4
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
with:
fetch-depth: 0
ssh-key: ${{ secrets.RELEASE_DEPLOY_KEY }}

- name: Install git-cliff
uses: taiki-e/install-action@v2
uses: taiki-e/install-action@ba47c86ac325773530516bb756137ac718732518 # v2
with:
tool: git-cliff

- name: Set up Python
uses: actions/setup-python@v5
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
with:
python-version: '3.12'

Expand All @@ -41,7 +160,7 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
python scripts/bump_version.py ${{ inputs.bump }}
python scripts/bump_version.py ${{ inputs.bump || needs.preflight.outputs.bump }}
VERSION=$(grep '^version = ' pyproject.toml | head -1 | cut -d'"' -f2)
echo "version=$VERSION" >> $GITHUB_OUTPUT

Expand All @@ -55,7 +174,7 @@ jobs:
git push && git push --tags

- name: Create GitHub Release
uses: softprops/action-gh-release@v2
uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2
with:
tag_name: v${{ steps.bump.outputs.version }}
generate_release_notes: true
Loading