Skip to content

feat(ci): weekly scheduled patch release trigger - #652

Open
fanhongy wants to merge 2 commits into
mainfrom
feat/weekly-patch-release-schedule
Open

fanhongy wants to merge 2 commits into
mainfrom
feat/weekly-patch-release-schedule

Conversation

@fanhongy

Copy link
Copy Markdown
Collaborator

Summary

Adds a schedule: trigger to release.yml for a weekly patch release
(Mondays 09:00 UTC), so releases don't depend on someone remembering to
click workflow_dispatch. See #651 for the motivating data (33 PRs merged
and unreleased since v2.4.1, including a few security-relevant fixes
sitting for 2-3 weeks).

Changes

  • schedule: cron: '0 9 * * 1' added alongside the existing
    workflow_dispatch trigger.
  • New "Check for unreleased commits" step (schedule-only) counts commits
    since the last tag; if zero, every downstream step is skipped via
    if: github.event_name != 'schedule' || steps.unreleased.outputs.count != '0'.
    No empty releases.
  • inputs.bump now falls back to 'patch' when the run has no inputs
    (i.e. was triggered by schedule, not workflow_dispatch).
  • Manual dispatch behavior, and the TestPyPI -> smoke-test ->
    maintainer-approval-gate pipeline downstream, are unchanged.

Testing

  • Validated YAML syntax locally.
  • Not yet exercised in CI — the schedule trigger and skip-guard logic
    haven't fired for real. Recommend a manual workflow_dispatch run
    first to confirm no regression on the existing path, and/or watching
    the first scheduled Monday run before trusting it unattended.

Open questions (see #651)

  • Is Monday 09:00 UTC the right day/time?
  • Should a security-labeled PR trigger an out-of-band release
    independent of the weekly schedule? (proposed as follow-up, not in
    this PR)

Closes #651

Add a schedule trigger to release.yml that fires a patch release
every Monday, gated by a check that skips the run entirely when
nothing has merged to main since the last tag. Manual
workflow_dispatch (patch/minor/major) is unchanged, and the
TestPyPI -> smoke-test -> maintainer-approval-gate pipeline is
untouched.

Closes #651
@fanhongy fanhongy added the enhancement New feature or request label Aug 21, 2026

@call-me-ram call-me-ram left a comment

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.

Thanks for taking this on — #651 is real (I count 33 commits sitting on main since v2.4.1) and the shape here is right: schedule alongside dispatch, empty-week skip, patch fallback, manual path untouched. I traced the skip expression through all three cases (dispatch, schedule+commits, schedule+empty) and it does what the description says: no empty releases, no version churn.

One blocker before this can run unattended, then a few cheap hardenings.

Blocker: nothing checks that CI passed on the SHA being released. Under workflow_dispatch a human looked at the badge before clicking; the cron removes that human. If main is red — or CI is still running — at Monday 09:00, this job will bump, tag, create the GitHub Release, and that Release fires publish-to-pypi, which pushes to TestPyPI with no environment protection at all (I checked: only the final pypi environment has required reviewers). By that point the tag and Release already exist, so the approver is choosing between shipping a red build and leaving a permanent version gap. Please add a schedule-only step before the bump that fails unless CI concluded green for the exact commit being released, e.g.:

- name: Require green CI on the release SHA
  if: github.event_name == 'schedule'
  env:
    GH_TOKEN: ${{ github.token }}
  run: |
    conclusion=$(gh run list --commit "$GITHUB_SHA" --workflow ci.yml \
      --json conclusion -q '.[0].conclusion')
    if [ "$conclusion" != "success" ]; then
      echo "CI is '$conclusion' at $GITHUB_SHA - refusing to release"; exit 1
    fi

Key it to $GITHUB_SHA, not the branch tip: for schedule events checkout pins to github.sha, the unreleased-count and the bump both run on that same checkout, and the push at the end is fast-forward-only, so gating on github.sha is race-free. Gating on "latest run on main" would reintroduce a check/release race. Side effect worth knowing: if Sunday-night CI is still in flight at 09:00, conclusion is null and the week is skipped — I think that's the right failure mode, but say so in a comment.

Cheap hardenings I'd like in this PR:

  1. if: github.repository == 'awslabs/cli-agent-orchestrator' on the job. Cron is disabled in forks by default, but a fork that enables Actions would happily tag and Release its own main every Monday. One line.
  2. concurrency: { group: release, cancel-in-progress: false } at workflow level. Today a manual dispatch overlapping the cron loses the race at git push and fails non-fast-forward — no double release, I checked the && chaining, but it's a guaranteed red run and confusing half-state. Serialize instead.
  3. git describe --tags --abbrev=0 --match 'v[0-9]*'. Every tag today is v*, but the first benchmark or RC tag someone pushes silently corrupts the unreleased count.

Two things I'll take as this-PR-or-fast-follow, your call:

  • The five copy-pasted step guards work, but the next person who adds a step to this job will forget the guard and it'll run on empty weeks. A check job exposing count plus a single job-level if: on the release job kills the whole class.
  • This workflow still uses mutable tags (taiki-e/install-action@v2, softprops/action-gh-release@v2, checkout, setup-python) while publish-to-pypi.yml and cargo-deny.yml are SHA-pinned with comments explaining exactly why. Making this job unattended-weekly, with the release deploy key sitting on disk from checkout while third-party actions run, is what changes the calculus — pin them to match the rest of the release path.

Last, a semantics note rather than a request: if a release fails midway — tag pushed but the Release step dies, or the Release exists but PyPI publish never completes — next Monday's run silently moves past it, leaving a version that never reached PyPI and no signal anyone should look. Fine to accept that for now, but write the decision down in the comment block, and I'd welcome a follow-up that checks the last tag actually landed on PyPI.

Also agree with your own testing note: once the blocker's in, do one manual dispatch to confirm no regression, and watch the first Monday run rather than trusting it cold.

@haofeif haofeif left a comment

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.

Reviewed at 070f7ec. The mechanics are right and I could not break them. I traced the skip expression through all three cases: dispatch always runs (the first clause short-circuits, so the empty steps.unreleased.outputs.count from the skipped step is never consulted), schedule-with-commits runs, schedule-with-zero-commits skips all five downstream steps so there is no tag, no Release, no version churn. inputs.bump || 'patch' does resolve to patch on a schedule event (the inputs context is null outside workflow_dispatch), and even if it resolved empty the run fails closed — bump_version.py exits 1 on a missing argument (scripts/bump_version.py:73). fetch-depth: 0 gives git describe the tags it needs. Manual dispatch is genuinely untouched, and all 24 checks are green.

Two blockers. One is @call-me-ram's, which I confirmed independently and can now put harder numbers on; the other has not been raised.

1. The forced patch bump breaks the semver contract this workflow itself writes. cliff.toml:6 emits "this project adheres to Semantic Versioning" into the CHANGELOG header, and cliff.toml:29 maps ^feat to the Added group. There are 13 feat: commits in the current unreleased window (36 commits since v2.4.1), so the first scheduled run publishes ## [2.4.2] with an ### Added block listing 13 features. That is not an edge case at 19-24 merged PRs/week — it is every week. The historical record agrees: every window that contained features got a minor bump (v2.1.0 3 feats, v2.2.0 12, v2.3.0 32, v2.4.0 22); the one clean patch, v2.4.1, had 0 feats. Details inline on the bump step.

2. Nothing checks CI on the SHA being released, and the surrounding configuration is weaker than the discussion above assumes. I checked the live settings: GET /repos/awslabs/cli-agent-orchestrator/branches/main/protection returns 404 "Branch not protected" — there are no required status checks on main at all — and the testpypi environment has "protection_rules": [], while only pypi carries required_reviewers. So a red Monday tip gets bumped, tagged, Released, and pushed to TestPyPI with zero human in the loop, and by the time anyone is asked to approve, the tag and the Release already exist. Concur with the blocker; inline with the shape I'd take.

The rest are P2s: no --match on git describe (which disagrees with cliff.toml's own tag_pattern), no completion check on the previous release, and no concurrency/repository guard.

One data point that upgrades @call-me-ram's "semantics note" from hypothetical to observed: the tag list and the shipped artifacts already disagree. v2.4.0 has a tag and a PyPI artifact but no GitHub Release (GET /releases/tags/v2.4.0 → 404, and it is absent from the paginated release list), and PyPI holds only 2.1.1, 2.3.0, 2.4.0, 2.4.1v2.2.0 was tagged and GitHub-Released on 2026-06-04 and never reached PyPI. Under workflow_dispatch a human was at least present when that happened. A cron makes those holes silent by construction.

Nits, no need to block on them: 0 9 * * 1 sits exactly at the top of the hour, which GitHub explicitly calls out as its high-load window for schedule — runs get delayed and occasionally dropped, and a dropped run here means a silently skipped release week, so shift it off the hour (17 9 * * 1). And seconding the pinning point with the concrete contrast — this file is on @v4/@v5/@v2 (lines 29, 47, 53, 79) while publish-to-pypi.yml SHA-pins every action with a version comment (lines 74, 78, 108, 118). Making this job unattended-weekly, with the release deploy key on disk while third-party actions run, is exactly the change that makes that gap matter.

Good change overall, and #651 is well-argued — the skip-guard and the untouched dispatch path are both done correctly. Just not safe to run unattended until the bump type is derived rather than assumed, and CI is gated.

Comment thread .github/workflows/release.yml Outdated
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
python scripts/bump_version.py ${{ inputs.bump }}
python scripts/bump_version.py ${{ inputs.bump || 'patch' }}

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] Derive the bump type from the commits instead of hardcoding patch

A scheduled run always bumps patch, but the window it releases is not patch-shaped. Since v2.4.1 there are 36 commits, 13 of them feat::

$ git log v2.4.1..origin/main --pretty=%s | grep -c '^feat'
13

cliff.toml:29 routes ^feat into the Added group, so the very first scheduled run writes this into CHANGELOG.md:

## [2.4.2] - 2026-08-24

### Added

- feat(tui): rewrite the cao tui front door in Rust (#547)
- feat(workflow): async run submit, discovery, and live event following (#505) (#525)
- feat(provider): add official xAI Grok CLI support (#596)
- feat: add MiniMax Code provider (#625)
  ... 9 more

…into a file whose header — generated by cliff.toml:6 — reads "this project adheres to Semantic Versioning". A patch release with an ### Added section of 13 features contradicts the sentence directly above it.

This isn't a rare window. At the merge volume cited in #651 (19-24 PRs/week) essentially every weekly window will contain features, and the project's own history confirms maintainers bump accordingly:

range commits feats bump
v2.0.2..v2.1.0 20 3 minor
v2.1.1..v2.2.0 42 12 minor
v2.2.0..v2.3.0 76 32 minor
v2.3.0..v2.4.0 79 22 minor
v2.4.0..v2.4.1 5 0 patch

The only patch release in that set is the only feature-free window. This PR replaces that judgement with a constant.

It also isn't fixable after the fact: once 2.4.2 is on PyPI carrying 13 features, consumers pinned ~=2.4.0 have silently taken them, and you cannot re-tag or yank your way back to a correct history.

Remedy — compute the bump from conventional commits in the same step that already knows the range:

- name: Determine bump type
  id: bumptype
  run: |
    if [ "${{ github.event_name }}" != 'schedule' ]; then
      echo "type=${{ inputs.bump }}" >> "$GITHUB_OUTPUT"; exit 0
    fi
    RANGE="${{ steps.unreleased.outputs.last_tag }}..HEAD"
    if git log "$RANGE" --pretty='%s%n%b' | grep -qE '^[a-z]+(\(.*\))?!:|^BREAKING CHANGE:'; then
      echo 'type=major' >> "$GITHUB_OUTPUT"
    elif git log "$RANGE" --pretty=%s | grep -qE '^feat'; then
      echo 'type=minor' >> "$GITHUB_OUTPUT"
    else
      echo 'type=patch' >> "$GITHUB_OUTPUT"
    fi

then pass ${{ steps.bumptype.outputs.type }} here. bump_version.py already accepts all three (scripts/bump_version.py:73), so no script change is needed — have the unreleased step also export last_tag so both steps agree on the range.

If you'd rather keep patch-only, that's a defensible product call, but then the semver claim in cliff.toml:6 has to go and #651 should say so explicitly — right now the workflow and the artifact it generates assert opposite things.

echo "count=$COUNT" >> "$GITHUB_OUTPUT"
if [ "$COUNT" -eq 0 ]; then
echo "No commits since $LAST_TAG - skipping scheduled release."
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.

Comment thread .github/workflows/release.yml Outdated
id: unreleased
if: github.event_name == 'schedule'
run: |
LAST_TAG=$(git describe --tags --abbrev=0)

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] git describe here disagrees with cliff.toml's own tag pattern

git describe --tags --abbrev=0 accepts any tag, but the changelog generator this workflow invokes two steps later already restricts itself:

# cliff.toml:41
tag_pattern = "v[0-9].*"

Today all 15 tags in the repo are v*, so the two agree and this is latent. The moment someone pushes a benchmark-2026-08 or an rc/foo tag, they diverge: the skip-guard measures from the stray tag while git-cliff measures from the last v*, so the count can read 0 (release skipped despite real commits) or the changelog can double-list a range the count thought was empty.

They must not be allowed to drift. Match the pattern that's already declared:

LAST_TAG=$(git describe --tags --abbrev=0 --match 'v[0-9]*')

While you're here, export it so a CI gate and a bump-type check can reuse the same range instead of recomputing it:

echo "last_tag=$LAST_TAG" >> "$GITHUB_OUTPUT"

Comment thread .github/workflows/release.yml Outdated
if: github.event_name == 'schedule'
run: |
LAST_TAG=$(git describe --tags --abbrev=0)
COUNT=$(git rev-list "${LAST_TAG}..HEAD" --count)

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] "Commits landed" is not the same question as "the last release shipped"

git rev-list "${LAST_TAG}..HEAD" asks only whether work has merged since the tag. It never asks whether that tag actually completed its pipeline. So a half-finished release is invisible to this guard, and the next Monday quietly advances the baseline past it.

That is not hypothetical here — the repository is already in that state twice:

$ gh api repos/awslabs/cli-agent-orchestrator/releases/tags/v2.4.0
{"message":"Not Found","status":"404"}          # tag exists, no GitHub Release

$ curl -s https://pypi.org/pypi/cli-agent-orchestrator/json | jq -r '.releases|keys[]'
2.1.1  2.3.0  2.4.0  2.4.1                       # v2.2.0 tagged + Released 2026-06-04, never on PyPI

v2.2.0 was tagged and GitHub-Released and never reached PyPI; v2.4.0 reached PyPI but has no GitHub Release. Under workflow_dispatch a human was at least in the room when each of those happened. A cron removes the only observer, and next Monday's count starts from the broken tag as if it were fine.

I'm not asking you to fix the pipeline in this PR. But since the cron is what makes the gap unobservable, please either:

  • add a cheap assertion after the count — curl -sf -o /dev/null "https://pypi.org/pypi/cli-agent-orchestrator/${LAST_TAG#v}/json" || { echo "::warning::${LAST_TAG} is not on PyPI - previous release may be incomplete"; } — as a warning first, promoted to a hard failure once you trust it; or
  • state the accepted behaviour in the comment block at lines 15-17, so the next reader knows a skipped-over broken release is a known trade-off rather than an oversight.

# commits" below).
- cron: '0 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.

Comment thread .github/workflows/release.yml Outdated
# Weekly patch release, Monday 09:00 UTC. Skipped automatically if
# nothing merged to main since the last tag (see "Check for unreleased
# commits" below).
- cron: '0 9 * * 1'

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.

[Nit] Move the cron off the top of the hour

GitHub documents that schedule events are delayed during high-load periods and specifically names the start of every hour as one, recommending a different minute; heavily delayed runs can be dropped entirely. 0 9 * * 1 sits exactly on that boundary.

For most workflows a dropped run is harmless. Here it is a silently skipped release week with no signal — the same class of invisible gap as the incomplete-release case. 17 9 * * 1 costs nothing.

Also worth noting in the comment block: cron is UTC-only and does not follow DST, so the run drifts an hour relative to local time twice a year. Fine if intentional, just easier for the next reader if it's written down alongside the day/time choice #651 asks about.

@fanhongy

fanhongy commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed both review rounds in 578fa98.

Summary:

  • Added a read-only preflight job and a single job-level release decision, eliminating the repeated per-step empty-week guards.
  • Scheduled releases now require a successful ci.yml run for the exact GITHUB_SHA; missing, pending, cancelled, or unsuccessful CI fails closed before any bump or tag.
  • Scheduled bump selection now follows conventional commits: breaking-change markers select major, feat selects minor, and other changes select patch. The current unreleased window selects major because commit e2e6318 contains an explicit BREAKING CHANGE: footer.
  • Stable release-tag selection accepts only v<major>.<minor>.<patch> tags.
  • The prior stable release must exist as both a GitHub Release and a PyPI version before a later scheduled release can proceed.
  • Added upstream-repository protection and non-cancelling release concurrency.
  • Moved the cron to Monday 09:17 UTC.
  • Pinned all actions in release.yml to immutable SHAs.
  • Documented the fail-closed recovery policy: a post-tag publishing failure requires maintainer recovery of that exact version and cannot be silently skipped by the next schedule.
  • Preserved the existing manual dispatch bump choices and behavior.

Verification:

  • YAML structure and embedded shell syntax checks passed.
  • Exact-SHA CI lookup returned completed:success.
  • The prior v2.4.1 GitHub Release and PyPI publication were verified.
  • The full PR check suite is green (24/24).

No files outside .github/workflows/release.yml were changed.

@haofeif haofeif left a comment

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.

Re-reviewed at 578fa98 (was 070f7ec).

All six of my earlier findings are genuinely fixed, and the rework is a real step up — I re-verified each one:

Prior finding Status
P1 hardcoded patch breaks the semver contract Fixed — bump is now derived from commit messages (:124-130)
P1 no CI-green gate Fixed — REQUIRED_WORKFLOWS=(ci.yml) must be completed:success at the exact SHA (:103-118). I confirmed ci.yml runs on push: branches: [main], so runs really do carry headSha == $GITHUB_SHA
P2 git describe matches any tag Fixed — git tag --merged "$GITHUB_SHA" --list 'v[0-9]*' + a strict ^v\d+\.\d+\.\d+$ filter (:61-66)
P2 no prior-release completion check Fixed — gh release view and a PyPI JSON lookup (:76-95). Nice catch that HTTPError subclasses URLError, so a 404 is caught
P2 no concurrency group / fork guard Fixed — concurrency: {group: release, cancel-in-progress: false} plus github.repository == on both jobs
Nit cron on the hour Fixed — 17 9 * * 1

Pinning the actions to commit SHAs was a good unprompted addition.

I'm still requesting changes for one reason, and it is a live one rather than a hypothetical: as written, the next scheduled run releases v3.0.0. Details inline at :124. I ran the workflow's own algorithm against the current repo state to confirm it, and traced how far that gets without a human. Everything else below is smaller.

Also: the PR title and #651 still say "weekly scheduled patch release trigger", which no longer describes the behaviour. Worth updating so the next reader isn't misled about what Monday does.

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.

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.

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.

@call-me-ram call-me-ram left a comment

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.

Re-reviewed at 578fa98 (was 070f7ec). All five of my 08-21 items are genuinely fixed — each verified at head, not from the diff:

Prior item Status
CI-green gate at the exact released SHA Fixed (:101-121) — gh run list --commit "$GITHUB_SHA" with a jq headSha == env.GITHUB_SHA re-filter, fail-closed on missing/pending/cancelled, and set -euo pipefail makes even a gh network failure abort. Race-checked: gate, both checkouts, the bump window, and the tag all pin to the same GITHUB_SHA, and git push is ff-only so a moved main aborts before any tag lands.
Repository guard Fixed — on both jobs (:32, :136-138), and release needs: preflight.
Concurrency group Fixed (:26-28, cancel-in-progress: false).
Tag selection Fixed stronger than asked (:62-66) — --list 'v[0-9]*' plus a strict ^v\d+\.\d+\.\d+$ filter that also excludes pre-releases, fail-closed when empty.
Mutable action tags Fixed — I dereferenced all four pins against their source repos: checkout=v4.4.0, setup-python=v5.6.0, gh-release=v2.6.2, install-action=2.86.5. (Nit: the last two # v2 comments could name the exact version.)

I also independently confirm @haofeif's v3.0.0 finding. Replaying the workflow's own derivation against current main (d697702): LAST_TAG=v2.4.1, 39 commits in the window, and exactly one major trigger — the BREAKING CHANGE: footer in e2e6318's body (the cao workflow run --json contract change from #525). It is a genuine footer, not a false positive, every prior-release preflight check passes, so the first scheduled Monday run really ships an unattended v3.0.0.

On the policy: I'd resolve it as refuse, not cap. Silently downgrading a breaking window to minor violates semver (a <3 pin would pull breaking code), which is worse than the problem. In the BUMP=major branch: print the commits carrying breaking markers and exit 1 — a red weekly run gets maintainer attention and points at workflow_dispatch with bump=major, where a human is present. ~4-line diff at :124-125. Auto-allowing feat→minor is fine to keep.

Keeping changes requested for one new finding from the fresh pass, plus a should-fix:

N1 (blocker): the scheduled pipeline can never complete a release to PyPI — and then deadlocks itself. publish-to-pypi.yml triggers on: release: [published], but this workflow creates the release with the default GITHUB_TOKEN (:177, no PAT/app token) — and GITHUB_TOKEN-initiated events do not trigger workflows. This is live behavior, not theory: the v2.3.0 and v2.4.1 releases produced zero release-event runs of publish-to-pypi; every successful PyPI publish since April was a manual workflow_dispatch after the fact. So: week 1, the schedule tags and creates a GitHub-only release; PyPI publish never fires; week 2, your own PyPI preflight (:84-99) fails closed and every subsequent scheduled run is red until someone manually dispatches publish (which also skips the smoke-test matrix, per publish-to-pypi.yml :219-223). Fail-closed, so nothing unsafe happens — but the feature as written delivers at most one unattended half-release and then permanently blocks, while the header comment (:15-19) presumes PyPI publishing follows. Fix options: create the release with an app/deploy token, or have release.yml invoke publish directly via workflow_call/gh workflow run.

N2 (should-fix): version-skip window between the two pushes at :174. If the branch push succeeds but git push --tags fails on a blip, the bump commit sits on main untagged; the next schedule still sees the old v2.4.1 in its checks, still sees the breaking commit, and bumps 3.0.0 → v4.0.0, skipping 3.0.0 — contradicting the :16-19 "a later schedule will not skip over it" comment, which only holds for failures after the tag lands. One-token fix: git push --atomic origin main "v$VERSION".

N3 (non-blocking): the major regex scans every body line (:123-124), so a revert or docs commit merely quoting BREAKING CHANGE: at start-of-line triggers an unattended major. Conventional-commits footers live in the final paragraph only. Low likelihood, but the blast radius is a major release; the refuse-on-major policy above also neutralizes most of the risk.

Checked and fine: PyPI lookup fails closed on network errors; no injection path from commit subjects/bodies (they never reach ${{ }} expressions, and the only interpolated preflight output is bump, constrained to three values); the one-item REQUIRED_WORKFLOWS allow-list is sound for this repo's trigger layout. Also +1 to haofeif's note that the PR title / #651 should stop saying "patch" — after the bump-derivation rework that's no longer what Monday does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automate weekly/fortnightly patch releases (currently ad hoc, workflow_dispatch-only)

3 participants