Handle breaking changes

.github/workflows/sync.yml

@@ -11,31 +11,30 @@ on:
     - cron: "17 * * * *" # hourly
   workflow_dispatch:
     inputs:
+      # auto: major if oasdiff or the API surface check finds a break,
+      # otherwise minor. major forces one for a break neither gate sees.
+      # There is no minor override: a detected break never ships as a minor.
       bump:
         description: "Which part of the version to bump"
         type: choice
-        options: [minor, major]
-        default: minor
-      acknowledge_breaking:
-        description: "Publish even though oasdiff reports a breaking change"
-        type: boolean
-        default: false
+        options: [auto, major]
+        default: auto
       dry_run:
         description: "Regenerate and build, but don't tag, release or publish"
         type: boolean
         default: false
 
 permissions:
   contents: write # commit + tag + create release
-  issues: write # open an issue when a breaking change is detected
+  issues: write # open an issue when a release is stuck
   id-token: write # PyPI trusted publishing (job-scoped, hence one job)
 
 concurrency:
   group: sync
   cancel-in-progress: false
 
 env:
-  # Pinned deliberately: oasdiff decides whether we publish at all.
+  # Pinned deliberately: oasdiff decides whether a release is a major.
   OASDIFF_VERSION: "1.32.1"
 
 jobs:
@@ -112,74 +111,75 @@ jobs:
 
           # 1 means it found breaking changes. Anything else means it couldn't
           # do the comparison at all — a truncated fetch, an HTML error page —
-          # which is not a breaking change and mustn't be filed as one.
+          # which is not a breaking change and mustn't be released as one.
           if [ "$rc" -ne 0 ] && [ "$rc" -ne 1 ]; then
             echo "::error::oasdiff exited $rc without comparing the schemas."
             exit 1
           fi
 
+          # A break does not stop the run: it makes this release a major.
+          # The report goes into the release notes so the major explains
+          # itself.
           if [ "$rc" -ne 0 ]; then
             echo "breaking=true" >> "$GITHUB_OUTPUT"
-            { echo "report<<OASDIFF_EOF"; echo "$report"; echo "OASDIFF_EOF"; } >> "$GITHUB_OUTPUT"
+            {
+              echo "### oasdiff: breaking changes to the API"
+              echo
+              echo '```'
+              echo "$report"
+              echo '```'
+              echo
+            } >> /tmp/breaks.md
           else
             echo "breaking=false" >> "$GITHUB_OUTPUT"
           fi
 
-      # A human dispatch with acknowledge_breaking skips this, and that is the
-      # only way to ship a major — nothing can publish outside this workflow.
-      - name: Halt on breaking change
-        if: steps.changed.outputs.changed == 'true' && steps.breaking.outputs.breaking == 'true' && !inputs.acknowledge_breaking
-        env:
-          GH_TOKEN: ${{ github.token }}
-          REPORT: ${{ steps.breaking.outputs.report }}
+      # Before the version is chosen, because a removal here makes this
+      # release a major.
+      - name: Regenerate and check the API surface
+        if: steps.changed.outputs.changed == 'true'
+        id: surface
         run: |
           set -euo pipefail
-          body="$(cat <<EOF
-          The upstream OpenAPI schema changed in a way that is **not backwards-compatible**, so the automated release was halted rather than publishing a breaking change as a minor bump.
-
-          A human needs to decide how to proceed — most likely a new major version.
-
-          <details><summary>oasdiff report</summary>
-
-          \`\`\`
-          $REPORT
-          \`\`\`
-
-          </details>
-
-          Triggered by run: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID
-          EOF
-          )"
-          # We stop without committing the new schema, so every later run sees
-          # the same diff and lands here again. Only file one issue.
-          existing="$(gh issue list --label breaking-change --state open --limit 1 --json number --jq '.[0].number // empty' 2>/dev/null || true)"
-          if [ -n "$existing" ]; then
-            echo "::error::Breaking change detected; issue #$existing is already open."
-            exit 1
+          mv openapi.json.new openapi.json
+          make generate
+          make install
+          # Exit 3 is removals only; anything else is a broken build and
+          # stops the run.
+          .venv-verify/bin/python scripts/verify_import.py api-surface.txt 2>/tmp/surface.txt && rc=0 || rc=$?
+          cat /tmp/surface.txt >&2
+          if [ "$rc" -eq 3 ]; then
+            echo "removed=true" >> "$GITHUB_OUTPUT"
+            {
+              echo "### Removed from the Python API"
+              echo
+              echo '```'
+              grep '^  ' /tmp/surface.txt
+              echo '```'
+              echo
+            } >> /tmp/breaks.md
+          elif [ "$rc" -ne 0 ]; then
+            exit "$rc"
+          else
+            echo "removed=false" >> "$GITHUB_OUTPUT"
           fi
 
-          # Created here rather than assumed: gh issue create fails outright on
-          # a label that doesn't exist, and an unlabelled fallback would be
-          # invisible to the dedup above, filing one issue an hour forever.
-          gh label create breaking-change \
-            --description "Schema changed incompatibly; release halted" \
-            --color B60205 2>/dev/null || true
-
-          gh issue create \
-            --title "Breaking API change detected — manual release required" \
-            --body "$body" \
-            --label "breaking-change"
-          echo "::error::Breaking change detected; opened an issue and stopped."
-          exit 1
-
-      # Scheduled runs only ever bump minor, since anything breaking has already
-      # stopped the run. Patch is for hand-fixes, major for a human dispatch.
+      # After the surface check, because it is one of the two signals. Patch
+      # is for hand-fixes and never cut here.
       - name: Decide next version
         if: steps.changed.outputs.changed == 'true'
         id: version
+        env:
+          BUMP_INPUT: ${{ inputs.bump || 'auto' }}
+          BREAKING: ${{ steps.breaking.outputs.breaking }}
+          REMOVED: ${{ steps.surface.outputs.removed }}
         run: |
           set -euo pipefail
-          BUMP="${{ inputs.bump || 'minor' }}"
+          if [ "$BUMP_INPUT" = "major" ] || [ "$BREAKING" = "true" ] || [ "$REMOVED" = "true" ]; then
+            BUMP=major
+          else
+            BUMP=minor
+          fi
           # Highest version, not `git describe`'s nearest-by-commit-distance:
           # a hand-cut patch tag would otherwise make us recompute a version
           # that already exists. The pattern also skips any non-semver tag,
@@ -200,10 +200,25 @@ jobs:
             exit 1
           fi
 
-          echo "Next version: $NEXT (base: ${BASE:-none}, bump: $BUMP)"
+          echo "Next version: $NEXT (base: ${BASE:-none}, bump: $BUMP, oasdiff break: $BREAKING, surface removals: $REMOVED)"
           echo "next=$NEXT" >> "$GITHUB_OUTPUT"
 
-      # Before make verify, so the wheel we check is the wheel we publish.
+          # Release notes: the breaks, if any, above GitHub's generated notes.
+          if [ "$BUMP" = "major" ]; then
+            {
+              echo "## Breaking changes"
+              echo
+              if [ -f /tmp/breaks.md ]; then
+                cat /tmp/breaks.md
+              else
+                echo "Released as a major by hand (bump: major); neither gate reported a break."
+              fi
+            } > /tmp/release-notes.md
+          else
+            : > /tmp/release-notes.md
+          fi
+
+      # Before make test, so the wheel we check is the wheel we publish.
       - name: Set the release version
         if: steps.changed.outputs.changed == 'true'
         env:
@@ -213,12 +228,14 @@ jobs:
           sed -i -E "0,/^version = /s/^version = .*/version = \"${NEXT#v}\"/" pyproject.toml
           grep -m1 '^version = ' pyproject.toml
 
-      - name: Regenerate and check
+      - name: Record the surface and test
         if: steps.changed.outputs.changed == 'true'
         run: |
           set -euo pipefail
-          mv openapi.json.new openapi.json
-          make generate
+          # Rewritten on every release, removals and additions included, so
+          # the next release checks against what this one ships. Before the
+          # tests, so the check inside make verify runs against it.
+          make surface
           # make test depends on make verify: builds the wheel, imports every
           # module, type-checks, checks the API surface, then runs the tests.
           # The generated code changes on every release, which is exactly when
@@ -229,7 +246,7 @@ jobs:
       # next run sees the same schema diff and fails identically. Left alone
       # that is a red cron every hour and no other signal.
       - name: Report a stuck release
-        if: failure() && steps.changed.outputs.changed == 'true' && steps.breaking.outputs.breaking != 'true'
+        if: failure() && steps.changed.outputs.changed == 'true'
         env:
           GH_TOKEN: ${{ github.token }}
         run: |
@@ -250,7 +267,7 @@ jobs:
 
           Run: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID
 
-          Likely causes: the generator rejected the new schema, a post-generation fix in \`scripts/\` stopped matching, or verification failed.
+          Likely causes: the generator rejected the new schema, a post-generation fix in \`scripts/\` stopped matching, or verification failed. Removals from the API surface do not land here: they make the release a major.
           EOF
           )"
 
@@ -264,6 +281,8 @@ jobs:
             echo
             echo "Would have released **$NEXT**."
             echo
+            cat /tmp/release-notes.md
+            echo
             echo "Built artefacts:"
             echo '```'
             ls -1 dist
@@ -288,7 +307,9 @@ jobs:
           # refused, and every later run recomputes this version and dies on
           # "tag already exists".
           git push --atomic origin HEAD "refs/tags/$NEXT"
-          gh release create "$NEXT" --generate-notes --title "$NEXT"
+          # --notes-file with --generate-notes puts the file above the
+          # generated notes. The file is empty on a minor.
+          gh release create "$NEXT" --generate-notes --notes-file /tmp/release-notes.md --title "$NEXT"
 
       # Has to live in this job: id-token is job-scoped, and a tag pushed with
       # GITHUB_TOKEN won't trigger a separate workflow.

Makefile

@@ -11,7 +11,7 @@ RUFF_VERSION := 0.16.8
 PACKAGE    := incident_io
 SCHEMA_URL := https://api.incident.io/v1/openapiV3.json
 
-.PHONY: fetch generate verify test surface clean
+.PHONY: fetch generate install verify test surface clean
 
 # -L because without it a redirect is a silent success writing zero bytes,
 # which parses as an empty schema. OUT lets the release workflow fetch to a
@@ -49,13 +49,18 @@ generate: openapi.json
 # halt the hourly pipeline rather than fail a pull request.
 MYPY_VERSION := 2.3.1
 
-# Python has no compile step, so we install the wheel and exercise it. See
-# scripts/verify_import.py for why importing the package alone isn't enough.
-verify:
+# Build the wheel and install it into .venv-verify, with what verify and test
+# run. Separate from verify so the release can run the surface check itself
+# and tell a removal (exit 3) from a broken build.
+install:
 	rm -rf dist
 	uv build
 	uv venv --clear .venv-verify
 	uv pip install --quiet --python .venv-verify dist/*.whl mypy==$(MYPY_VERSION) pytest pytest-asyncio
+
+# Python has no compile step, so we install the wheel and exercise it. See
+# scripts/verify_import.py for why importing the package alone isn't enough.
+verify: install
 	.venv-verify/bin/python scripts/verify_import.py api-surface.txt
 	# We ship py.typed, so consumers' type checkers read these annotations. Run
 	# against the installed package rather than the source tree: on the tree,
@@ -68,9 +73,9 @@ verify:
 test: verify
 	.venv-verify/bin/python -m pytest tests/ -q
 
-# Accept the current surface as the new baseline. Run after a deliberate
-# breaking change, alongside the major version bump.
-surface: verify
+# Accept the current surface as the new baseline, removals included. The
+# release runs this itself on every release.
+surface: install
 	.venv-verify/bin/python scripts/api_surface.py write $(PACKAGE) api-surface.txt
 
 clean:

README.md

@@ -122,8 +122,8 @@ default, so run with `-W default::DeprecationWarning` to see them.
 
 Releases are cut automatically whenever the API schema changes. We use
 [SemVer](https://semver.org/): additive API changes bump the minor version.
-Changes that would break existing code are never released automatically - they
-require a deliberate major version.
+Changes that would break existing code bump the major version, and the release
+notes list what broke.
 
 ## Support
 

scripts/api_surface.py

@@ -23,6 +23,10 @@
     python scripts/api_surface.py check incident_io api-surface.txt
 
 Run against the installed wheel, so it describes what consumers get.
+
+`check` exits 3 when the only problem is removed entries. Anything else that
+goes wrong exits 1. The release reads 3 as "this is a major" and 1 as a broken
+build.
 """
 
 from __future__ import annotations
@@ -117,11 +121,11 @@ def check(current: list[str], recorded_path: Path) -> int:
         print(f"  ... and {len(removed) - 40} more", file=sys.stderr)
     print(
         "\nConsumers import these by name, so removing one breaks them at import "
-        "or call time even when the wire contract is unchanged. This needs a major "
-        "version, not the automatic minor bump.",
+        "or call time even when the wire contract is unchanged. The release cuts "
+        "a major for it.",
         file=sys.stderr,
     )
-    return 1
+    return 3
 
 
 def main(argv: list[str]) -> int: