Skip to content

ci: guard the exported API against accidental breaking changes (v1) - #1304

Merged
kdroste-google merged 1 commit into
v1from
apidiff-ci-v1
Aug 11, 2026
Merged

ci: guard the exported API against accidental breaking changes (v1)#1304
kdroste-google merged 1 commit into
v1from
apidiff-ci-v1

Conversation

@baptmont

Copy link
Copy Markdown
Contributor

What

Backport of #1297 to the v1 maintenance branch. Adds an apidiff check that fails a pull request on incompatible changes to the exported API. Adding to the API is always allowed.

Both files are byte-identical to the ones on main.

Why here too

The workflow on main already lists v1 among its trigger branches, but a pull_request workflow runs from the head of the branch under test, so it could never fire on a v1 pull request until it existed on v1. Without this commit the branch is unguarded.

The guarantee also matters more here than on main: v1 is a maintenance line for the 1.x series, so any break in its exported API is by definition unintended.

What the check catches

The build and the tests do not catch every break. Adding a trailing variadic parameter to an exported function changes that function's type: every ordinary call site still compiles, so nothing goes red, while any caller that held the function as a value stops compiling. That reached a release on the 2.x line, and a test written specifically to pin backward compatibility did not catch it either, because a plain call expression is exactly the form a trailing variadic can never break.

Details worth reviewing

  • Compares against the merge base, not the target tip, so a branch cut before a recent change to v1 is not reported as having removed whatever v1 gained in the meantime.
  • One apidiff -m per module. Module mode reads every package in a module in a single load and skips internal/ on its own.
  • A module that cannot be read fails the check rather than passing quietly. A comparison that did not happen must not look like a comparison that found nothing, so a load failure is reported separately from a break and is not covered by the label below.
  • A deliberate break can be landed by labelling the pull request breaking-change. The comparison still runs and still prints what changed, so the break stays on the record; it just stops failing.

Testing Plan

Run end to end against this branch rather than a synthetic case:

  • Clean tree: exits 0, No incompatible changes., 16s.
  • Trailing variadic added to runner.New: reported as ./runner.New: changed from func(Config) (*Runner, error) to func(Config, ...any) (*Runner, error) and exits 1.
  • Same break with ALLOW_BREAKING=true: still printed, exits 0.
  • Module that does not compile: fails with the loader error surfaced, even with the label set.
  • The pinned apidiff version installs from a clean module; x/exp requires go 1.24.0, so the Go 1.25 toolchain v1 resolves from go.mod handles it.
  • Workflow YAML parses; the script passes bash -n.
Target upstream/v1, comparing against merge base 9fd38eae1f41
Collecting the API at the base revision
  google.golang.org/adk
Comparing HEAD against it
No incompatible changes.

real	0m16.353s

Generated with CloudCode, session ses_00f468febffe7dsyc1CNVAgbg7.

Backport of #1297 to the 1.x maintenance branch, where the guarantee it
enforces matters more than it does on main: v1 is a maintenance line, so
every break in it is by definition unintended.

The build and the tests do not catch every break. Adding a trailing
variadic parameter to an exported function changes that function's type:
every ordinary call site still compiles, so nothing goes red, while any
caller that held the function as a value stops compiling. That reached a
release, and a test written specifically to pin backward compatibility
did not catch it either, because a plain call expression is exactly the
form a trailing variadic can never break.

The check runs apidiff over every module and fails on incompatible
changes. Adding to the API is always allowed. A deliberate break can be
landed by labelling the pull request breaking-change, which still
compares and still prints what changed, so the break stays on the record.

The workflow already listed v1 among its trigger branches, but a
pull_request workflow runs from the head of the branch under test, so it
could never fire on a v1 pull request until it existed here. Both files
are byte-identical to the ones on main.

@kdroste-google kdroste-google left a comment

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.

LGTM

@kdroste-google
kdroste-google merged commit 3b6ad36 into v1 Aug 11, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants