feat: add opt-in freeze-on-ready to release-ready.yml#18
Conversation
Previously the release-ready job labeled + Slacked + set should_trigger_tests on the chore(main) PR before the freeze job drafted it, so the shadow PR was labeled (firing the test suites on it), announced as 'ready for review', and then converted to draft. Add a should_freeze path: when freeze-on-ready is on and the PR is on the primary branch, defer all of that to the frozen branch's release PR and only run the freeze.
|
@OpenHands We want to change from a workflow in agent-sdk repo, based on its own custom prepare-release skill, to a skill and human/agent workflow based on this repo’s release-please based action. Understand these. Find also the related SDK PR, that introduces release-please. Understand the prepare-release skill before and after the change introduced by BOTH these PRs. Human workflow:
After: Agent workflow:
After:
Answer us directly here on github. |
|
I'm on it! enyst can track my progress at all-hands.dev |
|
@enyst I traced both pieces:
Human workflowBeforeVerified. The current SDK release skill tells the agent/human to pick an explicit SemVer version and trigger That workflow:
I also checked recent SDK release PRs such as OpenHands/software-agent-sdk#3931 / #3888 / #3863: they have those three labels, and the corresponding labeled workflow runs exist for integration / behavior / examples. Then the human watches the release PR. If tests fail, they fix on the After, with SDK PR #3939 plus this freeze-on-ready PR enabledNo one asks the agent to create a release branch/PR anymore. Release-please keeps a draft release PR open automatically after pushes to The human workflow becomes:
If SDK PR #3939 is merged without this PR’s freeze option enabled, the “After” is weaker: marking the Concretely, SDK PR #3939 would need to update its caller from Agent workflowBefore, per the current SDK release skillThe agent is doing release orchestration around the custom workflow:
After, the skill should change to release-please orchestrationThe agent should no longer dispatch
So the biggest requested follow-up is not just workflow config: the SDK This comment was created by an AI agent (OpenHands) on behalf of the user. |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Notes as I’m reading the agent’s answer: HUMAN WORKFLOW
I think maybe we can open that as ready for review, or mark it automatically ready for review, once opened? So that the human doesn’t have to do it twice.
I’m a bit confused, why is the agent sometimes saying “X.Y” and other times “X.Y.Z”, will the human know which is which? Will we make multiple “X.Y” branches in time, for the same X and Y ? Does the agent know well enough the procedure, so that the human can say “once we merge the PR about abcd, backport it to the release branch”, and it will understand which release branch? |
|
AGENT WORKFLOW
I think the flow could be:
Should we switch it back to draft automatically, once the new branch/PR exists or was dispatched?
This is the second PR, the one we want; could we mark it ready automatically, there is no need to wait, right?
Why “guide”, should maybe the agent get some … command, in the skill, to do this merge back? |
This may be worth looking into, also…? @VascoSch92 I left above some notes on the steps. I would really love it if we can give the human a recipe or two recipes, and the agent is doing the rest… Maybe as commands? /release and /release-merge Tiny additional note: if I understand correctly, a PR title has “frozen” in it; maybe it’s worth considering to call it “current” if we can. Sorry to be a nitpicker! I’m trying to guess, looking from a human who doesn’t know the process in detail, what is the easiest and less error-prone wording we can give them? |
|
Correct me if I'm wrong, but aren't the releases in Is that the issue we're trying to solve? Or is this new functionality being introduced because the tests we invoke are time consuming and we want to minimize churn? I'm asking because I'd like to better understand what we're trying to accomplish before adding additional machinery that makes the |
Yes
No… During the life of the release PR, we run three workflows with tests (and we may want to add a security one). If tests fail, or deprecation deadlines, or compatibility checks : fix PR -> main -> cherry picked on release preparation branch. Sometimes, we re-run all tests; sometimes this way we don’t need to re-run tests; often, we may want to re-run some and not others. Yes they do take some time. And live LLM calls - all of them are live. If, on the other hand, in this time release-please keeps adding random other commits from main, well, then we’d always have to re-run all tests. That brings some question about costs, timing (hey please stop merging?) and for small releases, about stability e.g. maybe all we need was a small patch. |
jlav
left a comment
jlav
left a comment
There was a problem hiding this comment.
Thanks for the added context. I've got some additional questions, but the overall approach makes sense to me.
after merge, verify the GitHub Release/PyPI/binaries/downstream bump PRs, then guide the merge-back from release/X.Y to main and draft the announcement.
Right now, most repos are set to only allow squash merging PRs to main. If you squash merge the release-branch into, I don't think release-please will correctly diff the history between what's currently on main vs the last release tag, which was pointed at a commit that is only reachable from the release branch.
If tests fail, fix normally on main and cherry-pick/backport
Right now, the release actions are using github-style release notes, and github style release note derive the PR categorization from a label applied to each PR. I'd suggest
updating these instructions to say that a PR should be used to backport the fix, since I think most folks will interpret this to mean to run an actual git cherry-pick directly on the release branch, which won't show up in the release notes.
Given those issues I flagged above, I would suggest creating a minimal repo where you import this action from vasco/freeze-on-ready and actually try running through some release flows to make sure that it works as you expect and that the release notes it generates are actually accurate. When I first implemented these actions, I had an agent generate a bunch of fake PRs against a test repo and actually try creating a new release from main and doing patch releases between minor versions.
| from `main` and pull in everything that landed after your cut. You publish from the branch | ||
| instead, then merge back. | ||
|
|
||
| ### Automating the freeze on ready-for-review |
There was a problem hiding this comment.
It would be helpful to add step by step instructions to the readme that explain how to cut a release.
This section mostly includes technical details, so it's hard to understand how to actually use it in practice.
Called-workflow job permissions are validated statically at workflow startup, before any job runs and regardless of `if:` gating. The release-test-workflows job declares `actions: write` and the release-test-dispatch-event job declares `contents: write`, so the documented `read` grants killed every run as startup_failure — no jobs, no logs, only "This run likely failed because of a workflow file issue". Reproduced against @main with no freeze inputs at all, so this predates freeze-on-ready; it went unnoticed because release-ready.yml had never actually run in any repo.
The freeze job already tracked `created`, but never used it: when the freeze branch existed it skipped the createRef, then posted the same "Release frozen" comment and returned the PR to draft anyway. That reads as "I marked the PR ready and it silently un-readied itself, where is my release?" — which is exactly how it was hit. The PR is still returned to draft (merging from the moving base branch is still wrong), but the comment now says nothing was frozen, points at the branch that owns the release, and names the merge-back as the way out of the loop.
release-please reuses ONE release PR per branch and retitles it as the proposed version changes. Every freeze therefore left another comment on the same PR, each still saying "merge that one to publish" in the present tense about a release that had already shipped — so a PR titled "release 0.4.0" carried a comment pointing at a long-merged 0.3.1 cut. Tag each freeze comment with a hidden marker and delete earlier marked ones before posting, so exactly one — the current one — is ever present. Only comments carrying the marker AND authored by a bot are touched. Cleanup is best-effort: the branch is already cut by then, so failing to tidy up must not fail the freeze. Also name the version in the text; the branch alone (release/0.3) doesn't say which cut it described.
The marker only helps comments written from now on. Comments already sitting on a consumer's release PR — from any earlier ref of this workflow — were invisible to the cleanup and would stay there forever, which is the whole problem the marker was meant to solve. Match those by their distinctive heading as well, and only when the author is a Bot, so a human quoting a freeze comment is never deleted.
4 participants
Why
Consumers that keep release PRs as drafts (
draft-pull-request: true) use "markready for review" as the release-cut signal. But on the primary branch that
signal can't freeze the release: release-please rebases its PR onto the base
branch on every run, and merging the PR tags the base branch's tip — so anything
merged after "ready" still ships. The only true freeze is cutting the release
onto a
release/**branch (whichrelease.ymlalready runs on). This makes thatautomatic and opt-in.
Requested for the SDK adoption in OpenHands/software-agent-sdk#3939.
What
A new opt-in job in
release-ready.yml, mirroring the existingrelease-test-dispatch-event/release-test-workflowspattern (separate job,input-gated, own permissions). Default off, so existing consumers are untouched.
New inputs:
freeze-on-ready(boolean, defaultfalse)freeze-branch-prefix(string, defaultrelease/)When
freeze-on-ready: true, marking a release PR ready for review:release/<major.minor>at the PR's base SHA using the App token (aGITHUB_TOKENref wouldn't emit a push event, so release-please would neverrun on it), and
chore(main)PR to draft so it can't be merged from the movingbase branch — it stays the next-release preview.
release-please then opens the release PR on the frozen branch. Marking that
one ready runs the normal label/Slack/test path (the freeze job skips itself
because its base already starts with
freeze-branch-prefix).Notes for reviewers
chore(main)PR is returned to draft (preventsan accidental merge from the moving base — same
convertPullRequestToDraftused in OpenHands/OpenHands's
pr-readiness-confirm.yml). The alternative is tojust comment and leave it (today's "leave the shadow alone" guidance). Happy to
switch to comment-only or make it a third input if you prefer.
contents: write+pull-requests: writeissues: write; consumers grant these only when they opt in.path (real release PR +
ready_for_review) is not yet dogfooded — this repo'sown
pr-ready.ymlstill hasfreeze-on-readyoff. Suggest enabling it here (oran integration test) as a follow-up before a consumer relies on it.