ci: Release automation with release-please - #1398
Conversation
45c76cc to
9f4ec4a
Compare
karolpiotrowicz
left a comment
There was a problem hiding this comment.
This needs a rebase before it can land, and the rebase has to touch more than the file git flags. Four smaller notes are inline.
The manifest is stale in a way the merge conflict hides
v2.3.0 was tagged and released on 31 August, a week after this PR was opened, so main now carries const Version = "2.3.0". The only file git reports as conflicting is internal/version/version.go — but .github/.release-please-manifest.json is a new file, so it merges cleanly at 2.2.0, and last-release-sha still names the v2.2.0 commit. Resolving the visible conflict to 2.3.0 therefore leaves the two files that actually drive the release computation pointing a full release behind, with nothing flagging it.
That matters because release-please picks the previous release by matching the manifest value against release tags, rather than taking the newest tag — manifest.ts:577 is if (expectedVersion.toString() === tagName.version.toString()). With the manifest at 2.2.0 it skips v2.3.0 entirely and walks commits back to b264039, re-scanning everything already shipped. Two outcomes, both wrong:
- a
feat:anywhere in that range makes it propose2.3.0, and creating that release throwsDuplicateReleaseErrorbecause the tag already exists (github.ts:937, rethrown atmanifest.ts:1313-1315) - only
fix:/chore:in range makes it propose2.2.1, which tags a descendant ofv2.3.0and rewrites the constant from2.3.0down to2.2.1
Either way the first changelog re-lists every commit already in v2.3.0. The manifest, last-release-sha and the constant need to move together as one reviewed triple.
An absent RELEASE_PAT crashes the workflow rather than skipping it
You flagged creating the secret as a prerequisite, so this is only about the failure mode, which is harsher than it looks. token: ${{ secrets.RELEASE_PAT }} with no such secret does not fall back to github.token. The runner applies an action.yml default only when the key is absent from with: — ActionRunner.cs:214 is if (!inputs.ContainsKey(key)) — and an expression evaluating to "" still puts the key there. The action then reads it with required: true (index.ts:52), which throws on empty (core.ts:154).
So merging before the secret exists puts a red Release run on every push to main indefinitely. It blocks nothing, since the required checks are only lint (.) and test (.), but a permanently failing check on the default branch is its own cost. Worth sequencing the secret ahead of the merge, or letting the job tolerate the secret being absent.
For what it is worth, the parts most likely to go wrong here are right: the fork guard, top-level contents: read with the write grant scoped to the job, and passing the branch through env: instead of interpolating it into the run: block. actionlint is clean on the new workflow, and its repo-wide output is identical to main.
internal/version.Version is edited by hand and nothing fails when the bump is missed, so it drifts from the release tag: main shipped v2.1.0 and v2.2.0 while still reporting 2.0.0. Approach A fixes this with a bespoke workflow that pushes the bump to main, which needs github-actions[bot] on the branch-protection bypass list. Release Please gets the same result without that grant. It keeps one open release PR that rewrites the annotated constant, regenerates CHANGELOG.md and updates the manifest; merging that PR tags the merge commit, so a tag can only land on a commit whose constant already matches, and the bump goes through ordinary review. adk-python already releases this way, so this also aligns the two repos. Correct the constant to 2.2.0 (the current release) to seed the manifest, and document the flow, the conventional-commit requirement and the /v3 major-bump caveat in CONTRIBUTING.md. This is approach C of the version-drift RFC; approach A is on approach-a-release-workflow-bump and approach B on approach-b-derive-version-from-buildinfo.
adk-python stores both files there (.github/release-please-config.json and .github/.release-please-manifest.json, with a second -v1 pair for the maintenance line) and keeps the repository root clear of them. Match that layout so the two repos are configured the same way and the root stays reserved for Go module files.
The v1 branch is a separate release line on the unsuffixed google.golang.org/adk module path, with its own tags and its own copy of the version constant, so it cannot share main's manifest: one manifest holds one current version per package. Add .github/release-please-config-v1.json and .github/.release-please-manifest-v1.json, seeded at 1.6.0, which is where the v1 constant and the latest v1 tag already agree. The workflow now triggers on both branches, selects the pair from the branch it runs on, and passes target-branch explicitly so a v1 run cannot open a release PR against main. Concurrency is keyed per line so the two do not block each other. This mirrors adk-python, which keeps a -v1 config and manifest pair alongside the main ones and selects between them in release-cut.yml.
Forks inherit the workflow, so every push to a fork's main would start a release run: without RELEASE_PLEASE_TOKEN it fails, and with one it would try to cut a real release from the fork. Guard the job on the repository name, as adk-python's release-cut.yml does.
The pin was taken from refs/tags/v4's object SHA, but v4 is an annotated tag, so that SHA identifies the tag object rather than the commit it wraps. zizmor's ref-version-mismatch check flags the pin as disagreeing with its "# v4" comment, because the tag actually resolves to 5c625bfb5d1f. Pin that commit instead, which is also what adk-python pins.
…or-facing Drop the extra "deps" section: adk-go has no deps-typed commits, because dependabot writes chore(deps), which maps to the hidden chore section. Removing it leaves exactly the ten sections adk-python uses, in the same order, so the two repos produce comparably shaped release notes. Move the release mechanics out of CONTRIBUTING.md. Only maintainers cut releases, and the details (token setup, the v1 pair, the /vN major caveat) belong in the release playbook next to the rest of the procedure. What a contributor needs is one rule, so state it where PR requirements already live: titles are conventional commits, they become the squashed commit subject, and an unrecognised type is silently dropped from the notes and the version calculation.
adk-python stores the equivalent credential as RELEASE_PAT. Using the same name means whoever provisions or rotates it recognises it across both repositories, and the release playbook can describe one secret rather than two names for the same thing.
ed6cd5c to
00a4e38
Compare
workflow_dispatch accepts any ref, and the previous selector defaulted everything that was not v1 to the main line, so a dispatch from a feature branch would cut a release against main's config. Match main and v1 exactly and error otherwise. Also skip the release step, with a warning, when RELEASE_PAT is unset. An absent secret does not fall back to github.token: the empty value is still passed as an input and the action rejects it, so every push to main would carry a red run until the secret exists. Correct the pin comment to the exact patch (v4.4.1), matching how the other actions in the repo are pinned.
f755e9b to
9e8a2f0
Compare
The repo squash-merges with COMMIT_OR_PR_TITLE and allows rebase merges, so the PR title is not always what lands. Ask for the prefix on the commits too, rather than implying the title alone is enough.
9e8a2f0 to
9213d56
Compare
karolpiotrowicz
left a comment
There was a problem hiding this comment.
All five are fixed, and I checked each against the tree rather than taking the commit messages for it. The manifest, last-release-sha and the constant now agree on 2.3.0, with the sha being exactly the v2.3.0 commit, so the first run will compute from the right baseline. The conflict is gone and the PR is mergeable.
The token guard is the part I looked at hardest, since it is new code. It works: secrets genuinely is not available in a step if: and env is, so the mirror is the right shape. I confirmed that rather than assuming it — actionlint rejects secrets in a step if: and accepts env, which also means its clean run on this file is a real pass rather than a gap in what it checks.
Your reading of the v1 manifest is right, and the source backs it: release-please reads both the config and the manifest at the target branch (manifest.ts:445-450), so a v1 release bumps from v1's own files and main's copy drifting is inert. Your second option — one release-please-config.json and one manifest per branch, with no -v1 files on main — is the tidier end state, since it also deletes the branch-selector step. Not for this PR.
Two non-blocking notes inline, neither of which needs to hold up the merge.
On a PR-title lint: yes, worth doing, and the numbers support it. 9 of the last 80 commits on main carry no recognized type, including a feature (#1413) and a fix (#1390) — so roughly one in nine changes would be missing from the release notes today. Two things I would want from it: that it checks the commit subject as well on a single-commit PR, matching the wording you just landed, and that it takes its list of allowed types from release-please-config.json rather than a second hardcoded copy, so the two cannot drift. Separate PR.
One thing that is mine rather than yours: RELEASE_PAT still does not exist on the repo. I will create it. Until then the workflow lands inert, which is the point of the guard you added.
28abb6a
28abb6a to
c885a7a
Compare
Gate on whether RELEASE_PAT is set rather than on its value, so the job env carries a boolean instead of a repo-write credential that every step in the job inherits - including any step added later, far below the grant. No behavior change: the release step is still skipped, with the same warning, when the secret is absent.
c885a7a to
59f28f9
Compare
|
@karolpiotrowicz @wolo-lab Next steps after this PR is merged:
|
karolpiotrowicz
left a comment
There was a problem hiding this comment.
The token stays out of the job environment now, and the guard still behaves identically in both states: with no secret set, secrets.RELEASE_PAT != '' casts to the string "false", so the release step is skipped and the warning fires exactly as before. actionlint is clean on the file and the repo-wide output is unchanged from main.
Worth knowing what the first run will do once the secret exists. Seven commits have landed on main since v2.3.0 — one feat: (#724), five fix:, one chore:, nothing breaking — so release-please will open a chore(main): release 2.4.0 PR. That is a clean minor bump with no collision against an existing tag, which is the outcome the manifest fix was for.
Creating RELEASE_PAT is mine, not yours. Until it exists the workflow lands inert, which is what the guard is there for, so this does not need to wait on it.
Problem
internal/version.Versionis edited by hand before every release, and nothing failswhen that is missed - so it drifts.
mainshipped v2.1.0 and v2.2.0 while stillreporting
2.0.0. The value reaches users: it is sent to Gemini in thegoogle-adk/<version>header, attached to every OTel trace and log as theinstrumentation version, and reported as the MCP client version.
Solution
Adopt Release Please - tool adk-python already releases with - to maintain the constant instead.
On each push to a release line, Release Please keeps a single open release PR that
bumps the annotated constant, regenerates
CHANGELOG.md(optional) and updates the manifest.Merging that PR tags the merge commit and publishes the release, so a tag can only
land on a commit whose constant already matches.
mainandv1share one workflow but have separate config/manifest pairs, so the twolines advance independently.
This also corrects the constant to
2.3.0, the current release. The constant, themanifest and
last-release-shaare one triple and are set together: the manifest ishow release-please identifies the previous release (it matches the manifest value
against tags rather than taking the newest tag), so leaving it behind would make the
tool re-scan commits already shipped in
v2.3.0.Prerequisites (not in this PR)
RELEASE_PATsecret - a GitHub App token or PAT. Not required to open the releasePR (the action defaults
tokentoGITHUB_TOKEN); it exists because a PR authoredby
GITHUB_TOKENdoes not trigger other workflows, so the release PR would get noCI - and so could never satisfy the required checks. Without the secret the job
warns and skips, so merging ahead of it costs releases, not a red run on
main.v1(keeping its own1.6.0constant).That PR must also add the
x-release-please-versionannotation tointernal/version/version.goonv1: thegenericupdater rewrites only annotatedlines and skips the rest silently, so config alone would tag
v1.7.0while theconstant stayed
1.6.0. Until then onlymainreleases through this.CHANGELOG.mdor not (it is optional, Adk Python has it. But if we want it, we also should decide if the first changelog entry is the current release, or we populate it with previous releases also)Testing Plan
Validated end to end on a scratch repository using the same config.
go test -race -mod=readonly -count=1 -shuffle=on workgreenNotes
feat!:would propose v3.0.0, but a Go major bump alsorequires moving the module path to
/v3, which the tool cannot do.