Skip to content

feat(workflows): Add Claude AI SDK breaking change check workflow#36728

Open
KevinDavilaDotCMS wants to merge 1 commit into
mainfrom
36698-automate-min_sdk_version-bump-via-release-pipeline-ai-based-sdk-breaking-change-detection
Open

feat(workflows): Add Claude AI SDK breaking change check workflow#36728
KevinDavilaDotCMS wants to merge 1 commit into
mainfrom
36698-automate-min_sdk_version-bump-via-release-pipeline-ai-based-sdk-breaking-change-detection

Conversation

@KevinDavilaDotCMS

@KevinDavilaDotCMS KevinDavilaDotCMS commented Jul 25, 2026

Copy link
Copy Markdown
Member

Automate MinSdkVersion.VALUE maintenance via the release pipeline

Closes #36698

Problem

Follow-up to #36609 / #36678 (the SDK compatibility handshake). That work shipped
MinSdkVersion.java — a constant read by SdkVersionWebInterceptor and sent to every
@dotcms/* SDK consumer via the X-DotCMS-Min-SDK response header — but left its maintenance
fully manual: a developer had to remember to edit the constant by hand in their PR whenever a
change broke SDK compatibility ("Option A" from the original ticket: set it to whatever
@dotcms/client version was currently latest on npm at PR time).

That's fragile in two ways:

  • Nothing stops a developer from simply forgetting.
  • The "npm latest at PR time" heuristic has a known gap when the breaking change also needs an
    SDK-side fix — the version that's latest at PR time necessarily predates that fix.

This PR replaces the manual edit with a release-pipeline-driven mechanism with a safety net at
both ends: PR-time detection and release-time enforcement.

What this ships

  1. SDK Breaking Change label + AI detector (ai_claude-sdk-breaking-change.yml) — runs on
    every PR, evaluates the diff against a new reference doc, and labels PRs that break SDK
    compatibility.
  2. docs/core/SDK_BREAKING_CHANGE_CATEGORIES.md — the reference doc the AI (and human
    reviewers) use to judge what counts as SDK-breaking, grounded in the actual @dotcms/client
    surface (GraphQL page/content API, REST responses, the UVE postMessage protocol, the
    compatibility headers themselves).
  3. A new bump_min_sdk_version release input on cicd_6-release.yml — "does this release
    include an SDK-breaking change?"
  4. A release-time validation gate — fails the release outright if a merged PR since the last
    release carries the label but the operator left the checkbox unchecked.
  5. A post-release bump job — once the release fully succeeds, opens a PR (never a direct
    push) bumping MinSdkVersion.VALUE to the release version and pings Slack for human review.
  6. Updated Javadoc on MinSdkVersion.java describing this procedure.

Design decisions worth knowing about for review

Why the bump happens after the release succeeds, not during release-prepare.
The obvious-looking approach — folding the bump into release-prepare's existing automated
commit (the same one that already updates LICENSE and .mvn/maven.config) — is wrong for this
value specifically. That commit only ever lives on the disposable release-${version} branch,
which is never merged back to main. That's fine for LICENSE, because
update-license-date.sh recomputes the date fresh from "today" on every run — it never depends
on prior state. MinSdkVersion.VALUE is the opposite: it's a ratchet that must persist forward
across releases. If it only ever lived on the release branch, the next release-prepare run would
silently start again from main's stale value.

That in turn means timing matters: the bump can only safely happen once we know the release
shipped (build + deployment both green). If we bumped main eagerly in release-prepare and the
release then failed downstream, main would advertise a stricter compatibility floor for a
dotCMS version that customers never actually received — breaking currently-valid SDK installs for
nothing.

Why it's a PR, never a direct push to main.
Confirmed via the exact historical precedent for this same problem: the now-retired
cicd_manual-release-sdks.yml had to persist a version bump (core-web/libs/sdk/VERSION) onto
main after a release, and its solution was a dedicated "Open post-release PR to bump VERSION on
main" step — never a direct push. The PR it produced (#36563) was opened by github-actions[bot]
but merged by a human. There's no auto-merge anywhere in this repo's workflows to lean on instead.

Why one label, not four.
An earlier draft of this mechanism mirrored ai_claude-rollback-safety.yml's label scheme
exactly: AI: X / AI: Not X / Human: X / Human: Not X. For rollback-safety that split earns
its keep — a "clear stale AI labels on every push" preflight needs to not clobber a human's
deliberate override. For this mechanism we simplified to a single label, SDK Breaking Change:

  • The AI only ever adds it when it detects a break (with an explanatory PR comment). It never
    removes it.
  • Removing the label — because a human disagrees with the AI's verdict — is a plain manual
    action, available to anyone with write access. No separate Human: ... label needed.
  • A human can just as easily add the label manually — covering both a false negative from the
    AI, and PRs from external contributors, where the AI never runs at all (the org-membership
    security gate blocks it).
  • Net effect: no "stale label" preflight step needed, no risk of the AI clobbering a human
    decision on the next push, one label to reason about.

How it works end to end

1. PR time

Every PR triggers ai_claude-sdk-breaking-change.yml:

  • security-check — gates on dotCMS org membership (same mechanism as rollback-safety).
  • claude-sdk-breaking-change-check — reads docs/core/SDK_BREAKING_CHANGE_CATEGORIES.md, diffs
    the PR, and either:
    • posts a comment explaining which category matched + adds SDK Breaking Change, or
    • does nothing (no label, no comment) if it judges the change non-breaking.

A human reviewer can add or remove the label manually at any time regardless of the AI's verdict.

2. Release time — the checkbox is forgotten

Say a PR merged with the SDK Breaking Change label (correctly, from the AI or a human), two
more unrelated PRs merge after it, and then someone cuts a release without checking
bump_min_sdk_version. The very first job, verify-branch, runs a new step
(Validate bump_min_sdk_version against merged PR labels) that:

  • resolves the previous standard release tag,

  • pulls every PR merged since that tag (via squash-merge commit messages, same convention the
    existing release-qa-status tool relies on),

  • checks each one's labels,

  • and if any carries SDK Breaking Change while bump_min_sdk_version is false, fails the
    release immediately
    , before release-prepare even creates a branch or tag:

    ::error::This release (v26.10.01-1) includes PR(s) labeled 'SDK Breaking Change' (#36800) since v26.7.14-1, but 'bump_min_sdk_version' was left false. Re-run with bump_min_sdk_version=true, or remove the label from those PRs first if none of them actually break SDK compatibility.

The operator sees the error, realizes what happened, and re-runs with the checkbox checked.

If the checkbox is checked but no labeled PR is found in range, the release still proceeds — it
just prints a non-fatal warning, since a human explicitly opted in and may know something the
labels don't capture:

::warning::bump_min_sdk_version=true but no merged PR since v26.7.14-1 carries an SDK-breaking label. Proceeding, since a human explicitly opted in — but double-check this is intentional.

LTS releases (_lts_v##) skip this validation entirely — @dotcms/* SDKs track the @latest npm
cadence, not LTS patch cadence.

3. Release time — the release fails partway through

Say the checkbox was checked correctly, but the release fails during build or deployment
for unrelated reasons. The new bump-min-sdk-version job is gated with a strict success() on
needs: [release-prepare, build, deployment] — not the always() && !failure() && !cancelled()
idiom used by sibling jobs. If any of those three didn't fully succeed, this job doesn't run at
all
:

  • MinSdkVersion.VALUE is not touched.
  • No PR is opened.
  • No Slack message fires (the job never starts).

Nothing needs to be rolled back, because nothing was ever written. Once the operator fixes the
issue and re-runs the same release successfully, the bump job runs then instead.

4. Release time — the release succeeds

Once release-prepare, build, and deployment are all green, bump-min-sdk-version runs:

  • Checks out fresh main.

  • Idempotency check update with latest SVN #1: if MinSdkVersion.VALUE already equals this release's version
    (already-bumped, or a retry after a previous bump PR already merged), it no-ops — no duplicate
    commit, no duplicate PR.

  • Otherwise, creates branch sdk/bump-min-sdk-version-<version> off main, edits the constant,
    and verifies the edit actually took effect (guards against the VALUE = "..." literal
    silently failing to match if the file is ever reformatted — in which case it fails loudly
    instead of quietly opening an empty PR).

  • Commits, force-pushes the branch.

  • Idempotency check Test Branch and Commit #2: reuses an already-open PR for the same branch/version instead of
    opening a duplicate on a retried run.

  • Opens the PR (title: chore(sdk): bump MinSdkVersion.VALUE to <version>, body explains why),
    and posts to Slack (log-sdk-libs channel):

    :large_orange_circle: Attention dotters: dotCMS 26.10.01-1 was released with an
    SDK-breaking change.

    A PR bumping MinSdkVersion.VALUE to 26.10.01-1 is open on main.
    Please review and merge the post-release PR ASAP: [View PR]
    [View workflow run]

    If the job itself fails partway (e.g. gh pr create errors), a separate failure message goes
    to the same channel instead. The PR does not auto-merge — a human always reviews and merges
    it manually, same as the retired SDK-VERSION-bump precedent.

Test scenarios walked through during design (not yet live-fire tested — see Known limitations)

# Scenario Expected outcome Confirmed by
1 AI detects a breaking change; human agrees and leaves the label Label stays; release-time gate later catches it if the checkbox is forgotten Design walkthrough + exact error message traced above
2 AI flags a PR as breaking; human disagrees Human removes SDK Breaking Change manually; no separate label needed, no re-add risk since the AI never removes/re-adds on its own Confirmed the AI check only ever adds, per the workflow's own prompt instructions
3 AI misses a real breaking change (false negative), or PR is from an external contributor (AI never runs — blocked by the org-membership gate) A human adds SDK Breaking Change manually; the release-time gate treats it identically to an AI-applied label Confirmed via the gate's label check (grep -qxF 'SDK Breaking Change') — no distinction by origin
4 Dev forgets bump_min_sdk_version on a release that includes a labeled PR Release fails immediately in verify-branch, before any release branch/tag is created Exact ::error:: message traced above
5 Dev checks bump_min_sdk_version but release fails in build/deployment bump-min-sdk-version job doesn't run at all — nothing written, nothing to roll back Confirmed via the job's strict success() gate semantics (cross-checked against this repo's own promote-latest job, which relies on the same implicit behavior)
6 Release succeeds with the checkbox checked PR opens against main, Slack notification fires to log-sdk-libs Exact PR title/body and Slack message traced above
7 Same release re-run after a transient failure, or bump PR already merged No duplicate PR, no duplicate commit (idempotency checks #1 and #2) Traced through the exact grep/gh pr list dedupe logic
8 VALUE = "..." literal gets reformatted some day Job fails loudly with a clear error instead of silently opening an empty, misleading PR Locally simulated a reformatted file against the sed + verify logic — confirmed the mismatch is caught
9 Interaction with the existing next npm dist-tag publish job (cicd_3-trunk.yml) No interference — MinSdkVersion.java falls under the backend change-detection filter, not sdk_libs, so merging the bump PR never triggers publish-sdk-next Verified directly against .github/filters.yaml

Known limitations / accepted risk (not fixed here, flagged for awareness)

  • Two releases in flight at once, both flagged as SDK-breaking, would open two separate bump
    PRs (different branch names, keyed by version) — no collision, but a human merging both should
    merge the later version last.
  • A stale, unmerged bump PR from a previous release isn't caught by this run's own dedupe
    (different branch name). The validation step does emit a non-fatal ::warning:: if it finds an
    already-open sdk/bump-min-sdk-version-* PR against main, nudging the operator to merge it
    first.
  • Squash-merge dependency: both this mechanism's PR-label lookup and the existing
    release-qa-status tool assume squash-merge commit messages (... (#12345)) to map commits back
    to PR numbers. If the repo's merge policy ever changes, both silently stop finding PR numbers —
    fails safe (never blocks a release), but a real breaking PR could then slip through undetected.
    Pre-existing repo-wide assumption, not new to this PR.
  • This has not yet been live-fire tested against a real GitHub Actions run in this repo (only
    reasoned through carefully and unit-tested locally where possible — e.g. the sed/grep
    verification logic). A parallel attempt to validate this end-to-end in the disposable
    core-workflow-test fork hit an org-level branch-protection wall (a required status check that
    neither --admin merge nor a direct push could bypass with the available token permissions), so
    the AI-labeling behavior, the release-time gate, and the bump job's real GitHub Actions execution
    should be watched closely on this repo's first real usage.

Files changed

  • .github/workflows/cicd_6-release.yml — new bump_min_sdk_version input, new validation step
    in verify-branch, new bump-min-sdk-version job.
  • .github/workflows/ai_claude-sdk-breaking-change.yml (new).
  • docs/core/SDK_BREAKING_CHANGE_CATEGORIES.md (new).
  • dotCMS/src/main/java/com/dotcms/rest/config/MinSdkVersion.java — Javadoc updated to describe
    the automated procedure.

One-time setup already done

The SDK Breaking Change label has been created on this repo (color #b60205).

- Introduced a new GitHub Actions workflow to automatically check for breaking changes in SDK compatibility during pull requests.
- The workflow verifies if the user is a member of the dotCMS organization and logs the security decision.
- Implemented a job to analyze the pull request for SDK breaking changes based on defined categories, ensuring that any breaking changes are flagged appropriately.
- Updated the MinSdkVersion.java documentation to reflect the new automated bump procedure for SDK compatibility.
- Added a comprehensive reference document for SDK breaking change categories to guide developers in assessing changes.
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @KevinDavilaDotCMS's task in 8m 58s —— View job


Claude Code Review

  • Read changed files & diff
  • Review workflow YAML files
  • Review MinSdkVersion.java & docs
  • Post review

This is a well-structured feature (#36698). The bump-min-sdk-version job's idempotency guards, the sed-landed-verification, and the strict success() gate are all solid. Findings below are non-blocking.

New Issues

  • 🟡 Medium: .github/workflows/cicd_6-release.yml:148-150 — The safety gate extracts PR numbers with grep -oE '\(#[0-9]+\)$' over compare/${PREV_TAG}...main commit messages. Two ways this can silently miss a breaking PR and defeat the gate's entire purpose:

    1. The GitHub compare endpoint returns at most 250 commits in .commits (documented cap; --paginate does not lift it). If more than 250 commits merged since the last release, later PRs are invisible to this check.
    2. The regex only matches squash-merge subjects ending in (#N). A PR merged via a merge commit (Merge pull request #N from …) won't match.

    Result: a SDK Breaking Change-labeled PR could slip through with bump_min_sdk_version=false and the release ships without the floor bump — exactly the "silent gap" this feature closes. What to verify: whether dotCMS ever has >250 commits between standard releases, and that merges are always squash. If either is uncertain, prefer the list-commits endpoint or gh pr list --search "…is:merged" by merge date.

  • 🟡 Medium: .github/workflows/ai_claude-sdk-breaking-change.yml:83 — The prompt instructs git diff <base.sha>...<head.sha>. A three-dot diff requires base.sha to be present as a local object. If the reusable claude-orchestrator.yml@v3 checkout is shallow (default fetch-depth: 1) or only fetches the PR head, base.sha won't be reachable and git diff fails with fatal: bad object, silently producing an empty/failed analysis. Assumption: orchestrator checkout depth is unknown from this repo. What to verify: that the orchestrator fetches enough history (or fetch-depth: 0) for base.sha to resolve; otherwise use origin/main...HEAD or add an explicit git fetch origin <base.sha>.

  • 🟡 Medium: .github/workflows/ai_claude-sdk-breaking-change.yml:109 (and the read side at cicd_6-release.yml:155) — gh pr edit … --add-label "SDK Breaking Change" fails if the label doesn't already exist in the repo; gh does not auto-create labels. I found no definition of this label in the repo (.github/area-labels.yml etc.). What to verify: that SDK Breaking Change exists in repo Settings → Labels. If not, the AI's add-label step errors on the first real detection. (The release-side grep -qxF read is harmless if the label is missing — it just never matches.)

  • 🟡 Medium: .github/workflows/cicd_6-release.yml:311-425 + :469finalize now lists bump-min-sdk-version in needs, and the finalize phase marks the whole run FAILURE on any dependency result == "failure". Because the bump runs after build/deployment are already green and published, a failure creating the post-release PR (e.g. transient gh error) turns the entire release run red after the artifacts already shipped. That's misleading and could trigger unwarranted rollback investigation. The dedicated failure-Slack step already surfaces it — consider whether the bump PR failure should fail the aggregate run at all, or be continue-on-error/reported-only. (Skipped runs are handled correctly — result == "skipped" is not treated as failure.)

Notes (non-issue)

  • The if git diff --cached --quiet "nothing to commit" branch at cicd_6-release.yml:374-378 is effectively unreachable given the preceding CURRENT != RELEASE_VERSION guard, but if ever hit it falls through to git push --force + gh pr create with no diff, which errors. Harmless today; worth an exit 0 there for safety.
  • Doc docs/core/SDK_BREAKING_CHANGE_CATEGORIES.md is thorough and the H-1 self-referential warning (bumping VALUE to an LTS-shaped, non-numeric string silently disables compareVersions()) is a genuinely valuable callout.
    36698-automate-min_sdk_version-bump-via-release-pipeline-ai-based-sdk-breaking-change-detection

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

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : CI/CD PR changes GitHub Actions/workflows Area : Documentation PR changes documentation files

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Automate MIN_SDK_VERSION bump via release pipeline + AI-based SDK-breaking-change detection

2 participants