[ML] Add external changelog source support for release notes#145958
Open
edsavage wants to merge 13 commits intoelastic:mainfrom
Open
[ML] Add external changelog source support for release notes#145958edsavage wants to merge 13 commits intoelastic:mainfrom
edsavage wants to merge 13 commits intoelastic:mainfrom
Conversation
Extends the changelog bundling infrastructure to pull changelog entries from external repositories (initially elastic/ml-cpp) at bundle time. Changes: - changelog-schema.json: add optional source_repo field for entries from external repos - ChangelogEntry: add sourceRepo field and getRepoUrl() helper that returns the correct GitHub base URL for PR/issue links - BundleChangelogsTask: add ExternalChangelogSource config and fetch logic that clones external repo changelogs via git and merges them into the bundle with sourceRepo set automatically - ReleaseToolsPlugin: wire up elastic/ml-cpp as an external source - Templates (index.md, breaking-changes.md, deprecations.md): use change.repoUrl instead of hardcoded elastic/elasticsearch URLs This enables ml-cpp changelog YAML entries to appear in the ES release notes with correct PR links pointing to elastic/ml-cpp. Resolves: elastic/ml-cpp#2217 Made-with: Cursor
elasticsearchmachine
added
the
needs:triage
Contributor
WalkthroughThe bundle-changelogs task now accepts external changelog sources via a new Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as ReleaseToolsPlugin
participant Task as BundleChangelogsTask
participant Git as Git (external repo)
participant Parser as YAML Parser
participant Bundle as ChangelogBundle
Plugin->>Task: configure(externalSources)
Task->>Git: fetch(branchRef) for local repo
Git->>Parser: show/list local `*.yaml`
Parser->>Task: parsed local entries
loop for each ExternalChangelogSource
Task->>Git: shallow fetch(normalizeBranch)
Git->>Git: list `changelogPath` (`*.yaml`)
Git->>Parser: git show each YAML
Parser->>Task: parsed external entries (set sourceRepo)
Task-->>Task: merge entries
end
Task->>Task: sort by PR number
Task->>Bundle: create/write ChangelogBundle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
|
Pinging @elastic/ml-core (Team:ML) |
elasticsearchmachine
removed
the
needs:triage
Moves the external changelog source configuration from hardcoded Java to a YAML config file (external-changelog-sources.yml) in build-tools-internal/src/main/resources/. Adding or removing external repos is now a config-only change. Made-with: Cursor
Contributor
There was a problem hiding this comment.
Pull request overview
Extends the release-notes changelog bundling pipeline so it can pull per-PR YAML changelog entries from external repositories (initially elastic/ml-cpp) and render PR/issue links pointing at the correct GitHub repo.
Changes:
- Adds an optional
source_repofield to the changelog schema and introducesChangelogEntry.sourceRepo/repoUrlfor link generation. - Adds support in
BundleChangelogsTaskto fetch and merge changelog YAML files from configured external git repositories at bundle time. - Updates markdown templates to build PR/issue links from
${change.repoUrl}instead of hardcodingelastic/elasticsearch.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| build-tools-internal/src/main/resources/templates/index.md | Uses ${change.repoUrl} for PR/issue links in release notes. |
| build-tools-internal/src/main/resources/templates/breaking-changes.md | Uses ${change.repoUrl} for PR/issue links in breaking changes notes. |
| build-tools-internal/src/main/resources/templates/deprecations.md | Uses ${change.repoUrl} for PR/issue links in deprecations notes. |
| build-tools-internal/src/main/resources/external-changelog-sources.yml | Adds config listing external changelog sources (ml-cpp). |
| build-tools-internal/src/main/resources/changelog-schema.json | Adds optional source_repo field to schema. |
| build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.java | Loads external sources config and wires it into bundleChangelogs. |
| build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ChangelogEntry.java | Adds sourceRepo and getRepoUrl() helper for templates. |
| build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.java | Fetches external changelog YAMLs via git and merges entries into the bundle. |
Comments suppressed due to low confidence (1)
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.java:204
BundleChangelogsTaskdeclares an@OutputFileproperty (bundleFile) andReleaseToolsPluginsets it, butexecuteTask()writes the bundle to a hard-coded path (docs/release-notes/changelog-bundles/<version>.yml) instead of usingbundleFile(orchangelogBundlesDirectory). This makes the task’s declared outputs inconsistent with what it actually produces and can break Gradle’s output tracking/caching. Use the configured output properties when writing the bundle.
entries.sort(Comparator.comparing(ChangelogEntry::getPr));
ChangelogBundle bundle = new ChangelogBundle(version, finalize, Instant.now().toString(), entries);
yamlMapper.writeValue(new File("docs/release-notes/changelog-bundles/" + version + ".yml"), bundle);
} finally {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add @input annotation to externalSources property (required by Gradle) - Add Serializable to ExternalChangelogSource record (required for Gradle input fingerprinting) Made-with: Cursor
edsavage
force-pushed
the
feature/ml-cpp-changelog-integration
branch
from
9fafee9 to
fe4c7f8
Compare
- Add @JsonProperty("source_repo") to ChangelogEntry for correct snake_case YAML deserialization - Add ChangelogEntry.parse(String) overload to avoid temp file I/O - Replace persistent git remote with FETCH_HEAD-based fetch (no side effects on the local repo) - Add normalizeBranchForExternalFetch() to strip upstream/ prefixes and reject raw SHAs for external sources - Add ExternalChangelogSourceTests covering: YAML parsing with source_repo, default repo URL, branch normalization, SHA rejection Made-with: Cursor
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+306
to
+316
| private static String normalizeBranchForExternalFetch(String branchRef) { | ||
| if (branchRef.matches("^[0-9a-f]{7,40}$")) { | ||
| throw new IllegalArgumentException( | ||
| "Cannot use a commit SHA (" + branchRef + ") for external changelog sources. Use a branch name instead." | ||
| ); | ||
| } | ||
| int slashIndex = branchRef.indexOf('/'); | ||
| if (slashIndex >= 0) { | ||
| return branchRef.substring(slashIndex + 1); | ||
| } | ||
| return branchRef; |
There was a problem hiding this comment.
normalizeBranchForExternalFetch() currently strips everything up to the first '/' for any ref (e.g. "feature/foo" becomes "foo", and "refs/heads/main" becomes "heads/main"). That will break valid branch names containing slashes and common fully-qualified refs when fetching from external repositories. Consider only stripping known remote prefixes (e.g. "upstream/" and "origin/") rather than truncating on the first slash, and leave other refs untouched.
Only strip known remote prefixes (upstream/, origin/) instead of truncating at the first slash. Branch names like feature/foo and refs/heads/main are now passed through unchanged. Also made the method package-private (removed reflection from tests) and added test cases for slash-containing branch names. Made-with: Cursor
edsavage
force-pushed
the
feature/ml-cpp-changelog-integration
branch
from
a3791ae to
8eab7b7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.java`:
- Around line 248-250: The task currently aborts when a SHA-style --branch/ref
is passed because normalizeBranchForExternalFetch() throws; instead detect SHA
refs in fetchExternalChangelogs(ExternalChangelogSource source, String
branchRef) and early-return an empty List (skip external fetching) when
branchRef matches a commit SHA pattern (e.g., 40 hex chars or common shorter SHA
lengths), leaving local SHA handling intact; update the same check where
external fetch is invoked later (the other fetch call around the 299-303 area)
so both code paths skip external sources for SHA refs rather than calling
normalizeBranchForExternalFetch() and failing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 800b2eca-8e53-44f8-a3ed-c47e75d50835
📒 Files selected for processing (2)
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.javabuild-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ExternalChangelogSourceTests.java
🚧 Files skipped from review as they are similar to previous changes (1)
- build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ExternalChangelogSourceTests.java
When --branch is a commit SHA (valid for ES-local changelogs), skip external changelog fetching instead of aborting the task. This preserves the existing SHA workflow while external sources are simply omitted from that particular bundle. Made-with: Cursor
edsavage
force-pushed
the
feature/ml-cpp-changelog-integration
branch
from
1c0f1a1 to
ea5cf1b
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.java (1)
303-305: Consider case-insensitive SHA detection.The regex
^[0-9a-f]{7,40}$only matches lowercase hexadecimal. Git SHAs from some tools or clipboard operations may be uppercase or mixed-case. If this is intentional (requiring callers to normalize), document it; otherwise consider case-insensitive matching.Option: Case-insensitive matching
static boolean isShaRef(String ref) { - return ref.matches("^[0-9a-f]{7,40}$"); + return ref.matches("(?i)^[0-9a-f]{7,40}$"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.java` around lines 303 - 305, The isShaRef method currently only matches lowercase hex; update it to accept mixed- or upper-case SHAs by making the regex case-insensitive (e.g., use a case-insensitive flag or Pattern.CASE_INSENSITIVE) or normalize the ref (e.g., toLowerCase()) before matching; adjust the implementation in isShaRef(String ref) and add a brief comment stating that SHA matching is case-insensitive.build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ExternalChangelogSourceTests.java (2)
91-102: Test method name is misleading.This test verifies that record accessor methods return constructor-provided values, which is implicit behavior for Java records. The name
testExternalChangelogSourceSerializationsuggests it testsjava.io.Serializableserialization/deserialization round-tripping.Consider renaming to
testExternalChangelogSourceAccessorsor removing the test if accessor correctness is considered trivial for records.Suggested rename
`@Test` - public void testExternalChangelogSourceSerialization() { + public void testExternalChangelogSourceAccessors() { var source = new BundleChangelogsTask.ExternalChangelogSource(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ExternalChangelogSourceTests.java` around lines 91 - 102, The test method name testExternalChangelogSourceSerialization is misleading because it only asserts the record accessors of BundleChangelogsTask.ExternalChangelogSource; rename the test to something like testExternalChangelogSourceAccessors (or remove the test if deemed redundant) and update any references accordingly so the test name reflects that it verifies repoUrl(), sourceRepo(), and changelogPath() return the constructor values.
81-89: Consider testing uppercase hex SHA handling.The
isShaRefimplementation uses[0-9a-f]which only matches lowercase hex. While Git typically outputs lowercase SHAs, some tools or copy-paste scenarios may produce uppercase. Adding a test case would document the expected behavior.Optional: Add uppercase test case to document behavior
`@Test` public void testIsShaRef() { assertThat(BundleChangelogsTask.isShaRef("abc1234"), equalTo(true)); assertThat(BundleChangelogsTask.isShaRef("a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"), equalTo(true)); assertThat(BundleChangelogsTask.isShaRef("main"), equalTo(false)); assertThat(BundleChangelogsTask.isShaRef("9.3"), equalTo(false)); assertThat(BundleChangelogsTask.isShaRef("upstream/main"), equalTo(false)); assertThat(BundleChangelogsTask.isShaRef("feature/foo"), equalTo(false)); + // Uppercase hex is not detected as SHA (by design - normalize to lowercase before checking) + assertThat(BundleChangelogsTask.isShaRef("ABC1234DEF5678"), equalTo(false)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ExternalChangelogSourceTests.java` around lines 81 - 89, Add a test case in ExternalChangelogSourceTests.testIsShaRef to cover uppercase hex SHAs: call BundleChangelogsTask.isShaRef with an uppercase SHA (e.g., 40 hex chars A-F and 0-9) and assert the expected boolean result to document behavior; reference the existing testIsShaRef method and BundleChangelogsTask.isShaRef to locate where to add the new assertion and decide whether the expected outcome should be true (if you want to accept uppercase) or false (if you want to document rejection).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.java`:
- Around line 303-305: The isShaRef method currently only matches lowercase hex;
update it to accept mixed- or upper-case SHAs by making the regex
case-insensitive (e.g., use a case-insensitive flag or Pattern.CASE_INSENSITIVE)
or normalize the ref (e.g., toLowerCase()) before matching; adjust the
implementation in isShaRef(String ref) and add a brief comment stating that SHA
matching is case-insensitive.
In
`@build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ExternalChangelogSourceTests.java`:
- Around line 91-102: The test method name
testExternalChangelogSourceSerialization is misleading because it only asserts
the record accessors of BundleChangelogsTask.ExternalChangelogSource; rename the
test to something like testExternalChangelogSourceAccessors (or remove the test
if deemed redundant) and update any references accordingly so the test name
reflects that it verifies repoUrl(), sourceRepo(), and changelogPath() return
the constructor values.
- Around line 81-89: Add a test case in
ExternalChangelogSourceTests.testIsShaRef to cover uppercase hex SHAs: call
BundleChangelogsTask.isShaRef with an uppercase SHA (e.g., 40 hex chars A-F and
0-9) and assert the expected boolean result to document behavior; reference the
existing testIsShaRef method and BundleChangelogsTask.isShaRef to locate where
to add the new assertion and decide whether the expected outcome should be true
(if you want to accept uppercase) or false (if you want to document rejection).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ad78d2ed-aef7-459a-b9f9-e679382b4e6e
📒 Files selected for processing (3)
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/BundleChangelogsTask.javabuild-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.javabuild-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ExternalChangelogSourceTests.java
🚧 Files skipped from review as they are similar to previous changes (1)
- build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.java
- Make isShaRef case-insensitive to handle uppercase/mixed-case SHAs - Rename testExternalChangelogSourceSerialization to testExternalChangelogSourceAccessors (more accurate name) - Add test cases for uppercase and mixed-case SHA detection Made-with: Cursor
edsavage
force-pushed
the
feature/ml-cpp-changelog-integration
branch
from
df79cc9 to
10aa45f
Compare
Tests for sourceRepo propagation via setSourceRepo, invalid YAML parsing, and isShaRef boundary conditions. Made-with: Cursor
Collaborator
|
Pinging @elastic/es-delivery (Team:Delivery) |
ApprovabilityVerdict: Needs human review This PR introduces a new feature that fetches changelog entries from external repositories during build time. The author does not own any of the 9 modified files (all owned by @elastic/es-delivery), which warrants review by the designated code owners. Additionally, new features introducing new capabilities should receive human review. You can customize Macroscope's approvability policy. Learn more. |
breskeby
approved these changes
Apr 21, 2026
Contributor
breskeby
left a comment
breskeby
left a comment
There was a problem hiding this comment.
lgtm. one nit issue about changelog entry ordering
| } | ||
| } | ||
|
|
||
| entries.sort(Comparator.comparing(ChangelogEntry::getPr)); |
Contributor
There was a problem hiding this comment.
nit: With multiple repos, entries sharing a PR number have undefined ordering. Adding sourceRepo as a secondary sort key would make output deterministic.
breskeby
reviewed
Apr 21, 2026
| * All other refs (including branch names with slashes like {@code feature/foo}) | ||
| * are passed through unchanged. | ||
| */ | ||
| static boolean isShaRef(String ref) { |
Contributor
There was a problem hiding this comment.
the javadoc does not match and I guess is meant for the method below
breskeby
approved these changes
Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the changelog bundling infrastructure to pull changelog entries from external repositories at bundle time. Initially configured for
elastic/ml-cpp, this enables ml-cpp changelog YAML entries to appear in the Elasticsearch release notes with correct PR links.Motivation
Currently, ml-cpp changes are manually copied into
CHANGELOG.asciidocat release time — a process that's error-prone, creates merge conflicts, and misses entries. The ml-cpp team has adopted the ES per-PR YAML changelog format (identical schema) and needs a way for those entries to flow into the ES release notes automatically.This resolves elastic/ml-cpp#2217, originally filed by @pugnascotia when the changelog system was created.
Changes
Schema
changelog-schema.json: added optionalsource_repofield (string, e.g.elastic/ml-cpp). Existing entries without this field continue to work — it defaults toelastic/elasticsearch.ChangelogEntry
sourceRepofield with@JsonProperty("source_repo")for correct snake_case YAML deserialization.getRepoUrl()helper that returnshttps://github.com/<source_repo>, defaulting toelastic/elasticsearchwhen null.parse(String)overload for parsing YAML directly from a string (used by external changelog fetching to avoid temp file I/O).BundleChangelogsTask
ExternalChangelogSourcerecord (implementsSerializablefor Gradle@Inputfingerprinting) withrepoUrl,sourceRepo, andchangelogPathfields.fetchExternalChangelogs()method that:upstream/prefixes, rejects raw SHAs)git fetchintoFETCH_HEAD(no persistent remote added — no side effects on the local checkout)git ls-treegit showand parses directly from the string contentsourceRepoon each parsed entry automaticallynormalizeBranchForExternalFetch()to strip remote prefixes and reject SHAs that are meaningless for external repos.@Inputfor proper Gradle up-to-date checking.Configuration
build-tools-internal/src/main/resources/external-changelog-sources.yml— adding or removing external repos is a config-only change, no Java modifications needed:ReleaseToolsPluginloads this config at task configuration time via Jackson YAML.Templates
index.md,breaking-changes.md,deprecations.md: replaced hardcodedhttps://github.com/elastic/elasticsearchURLs with${change.repoUrl}so PR/issue links point to the correct repository. ES entries are unaffected (default URL iselastic/elasticsearch).Tests
ExternalChangelogSourceTests: 5 test cases covering:source_repofieldsource_repois absentExternalChangelogSourcerecord constructionE2E verification
Tested locally with a test git repo containing ml-cpp changelog entries:
./gradlew bundleChangelogs --branch main— fetched 3 ml-cpp entries viaFETCH_HEAD, merged into the 9.4.0 bundle withsourceRepo: elastic/ml-cpp./gradlew generateReleaseNotes— rendered ml-cpp entries with correct links:elastic/elasticsearchlinks as before.Example
An ml-cpp changelog entry:
Renders in the release notes as:
Test plan
compileJavapassesReleaseNotesGeneratorTestand existing template tests pass (backward compatible)validateChangelogspasses (existing ES entries validate against updated schema)ExternalChangelogSourceTestspass (5 new test cases)bundleChangelogs --branch mainincludes ml-cpp entries withsourceRepogenerateReleaseNotesrenders correct repo-specific PR linksRelated