From d066060c1cb751e547528b15ca0ff5b50d958956 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 17 Aug 2026 03:43:34 +0000 Subject: [PATCH 1/5] build(release): separate sync changelog into standalone workflow and CLI tool Currently, when a backport is made to a release branch by the release tool and workflow, the same job and logic tries to create the corresponding sync PR to main. This couples backport processing with changelog synchronization, preventing the changelog sync from being triggered or retried independently. Separate changelog synchronization into a dedicated `release sync-changelog` CLI command and a standalone GitHub Actions workflow that can run automatically or be manually dispatched. Update the backport processing logic and workflow to record the sync changelog task without executing it directly. --- .github/workflows/on_comment.py | 4 + .github/workflows/on_comment.yaml | 9 + .github/workflows/on_pr_closed.yaml | 32 ++ .../workflows/release_process_backports.yaml | 9 + .github/workflows/release_sync_changelog.yaml | 67 ++++ RELEASING.md | 23 ++ tests/tools/private/release/BUILD.bazel | 12 + .../private/release/process_backports_test.py | 276 +------------ .../private/release/sync_changelog_test.py | 361 ++++++++++++++++++ tests/workflows/on_comment_test.py | 13 + tools/private/release/process_backports.py | 254 +----------- tools/private/release/release.py | 2 + tools/private/release/sync_changelog.py | 303 +++++++++++++++ 13 files changed, 850 insertions(+), 515 deletions(-) create mode 100644 .github/workflows/release_sync_changelog.yaml create mode 100644 tests/tools/private/release/sync_changelog_test.py create mode 100644 tools/private/release/sync_changelog.py diff --git a/.github/workflows/on_comment.py b/.github/workflows/on_comment.py index 1aa0f1baa0..2672db3cd1 100755 --- a/.github/workflows/on_comment.py +++ b/.github/workflows/on_comment.py @@ -101,6 +101,10 @@ def _process_release_issue_comment( _write_github_output("command", "process-backports") return + if _match_command(("sync-changelog", "sync_changelog"), comment_body): + _write_github_output("command", "sync-changelog") + return + if m := _match_command(("backport", "backports"), comment_body): raw_args = m.group(1) if m.group(1) else "" items = [item for item in re.split(r"[\s,]+", raw_args) if item] diff --git a/.github/workflows/on_comment.yaml b/.github/workflows/on_comment.yaml index e94c382f26..4620fe8ccf 100644 --- a/.github/workflows/on_comment.yaml +++ b/.github/workflows/on_comment.yaml @@ -108,6 +108,15 @@ jobs: comment_id: "${{ github.event.comment.id }}" secrets: inherit + call_sync_changelog: + needs: parse_comment + if: needs.parse_comment.outputs.command == 'sync-changelog' + uses: ./.github/workflows/release_sync_changelog.yaml + with: + issue: ${{ needs.parse_comment.outputs.issue_number }} + comment_id: "${{ github.event.comment.id }}" + secrets: inherit + call_promote: needs: parse_comment if: needs.parse_comment.outputs.command == 'promote' diff --git a/.github/workflows/on_pr_closed.yaml b/.github/workflows/on_pr_closed.yaml index 94c49b6561..e21864769e 100644 --- a/.github/workflows/on_pr_closed.yaml +++ b/.github/workflows/on_pr_closed.yaml @@ -82,6 +82,38 @@ jobs: --remote origin \ --no-dry-run + sync_changelog: + needs: process_backports + runs-on: ubuntu-latest + permissions: + contents: write + issues: write + pull-requests: write + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Setup Bazel + uses: bazel-contrib/setup-bazel@0.19.0 + with: + bazelisk-version: 1.20.0 + + - name: Configure Git Identity + run: | + git config --global user.name "github-actions[bot]" + git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com" + + - name: Sync Changelog + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + bazel run //tools/private/release -- sync-changelog \ + --remote origin \ + --no-dry-run + + complete_sync_changelog: if: | github.event.pull_request.merged == true && diff --git a/.github/workflows/release_process_backports.yaml b/.github/workflows/release_process_backports.yaml index ca719778df..c613c5a3ca 100644 --- a/.github/workflows/release_process_backports.yaml +++ b/.github/workflows/release_process_backports.yaml @@ -74,3 +74,12 @@ jobs: --remote origin \ --no-dry-run \ "${ARGS[@]}" + + call_sync_changelog: + needs: process_backports + uses: ./.github/workflows/release_sync_changelog.yaml + with: + issue: ${{ inputs.issue }} + comment_id: ${{ inputs.comment_id }} + secrets: inherit + diff --git a/.github/workflows/release_sync_changelog.yaml b/.github/workflows/release_sync_changelog.yaml new file mode 100644 index 0000000000..e095810486 --- /dev/null +++ b/.github/workflows/release_sync_changelog.yaml @@ -0,0 +1,67 @@ +name: "Release: Sync Changelog" + +on: + workflow_dispatch: + inputs: + issue: + description: 'The Release Tracking Issue Number (e.g., 142)' + required: false + type: string + comment_id: + description: 'The ID of the comment that triggered this run (optional)' + required: false + type: string + workflow_call: + inputs: + issue: + description: 'The Release Tracking Issue Number (e.g., 142)' + required: false + type: string + comment_id: + description: 'The ID of the comment that triggered this run (optional)' + required: false + type: string + +permissions: + contents: write + issues: write + pull-requests: write + +jobs: + sync_changelog: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Setup Bazel + uses: bazel-contrib/setup-bazel@0.19.0 + with: + bazelisk-version: 1.20.0 + + - name: Configure Git Identity + run: | + git config --global user.name "github-actions[bot]" + git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com" + + - name: Sync Changelog to Main + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COMMENT_ID: ${{ inputs.comment_id }} + ISSUE: ${{ inputs.issue }} + run: | + ARGS=() + if [ -n "$ISSUE" ]; then + ISSUE="${ISSUE#\#}" + ARGS+=("--issue=$ISSUE") + fi + if [ -n "$COMMENT_ID" ]; then + ARGS+=("--triggering-comment=$COMMENT_ID") + fi + + bazel run //tools/private/release -- sync-changelog \ + --remote origin \ + --no-dry-run \ + "${ARGS[@]}" diff --git a/RELEASING.md b/RELEASING.md index 318434cc1a..c166d06fdf 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -162,6 +162,29 @@ bazel run //tools/private/release -- process-news 2.3.0 news/3997.added.md bazel run //tools/private/release -- process-news 2.3.0 3997 ``` +### Syncing Changelog to Main + +When backports are processed, a separate workflow and job creates a sync PR to +`main` to merge news entries into `CHANGELOG.md` and update `VERSION_NEXT_*` +placeholders. + +You can also manually trigger changelog syncing using the GitHub CLI or Actions +UI: + +```shell +gh workflow run release_sync_changelog.yaml \ + --repo bazel-contrib/rules_python \ + -f issue= +``` + +Or comment `/sync-changelog` on the release tracking issue, or run via the +release tool CLI: + +```shell +bazel run //tools/private/release -- \ + sync-changelog --issue --remote origin --no-dry-run +``` + ### Failure Behavior If a backport fails to process (e.g., due to cherry-pick conflicts): * The failed backport checklist item will remain unchecked with diff --git a/tests/tools/private/release/BUILD.bazel b/tests/tools/private/release/BUILD.bazel index e69e190506..4f94c9b9cc 100644 --- a/tests/tools/private/release/BUILD.bazel +++ b/tests/tools/private/release/BUILD.bazel @@ -76,6 +76,18 @@ pytest_test( ], ) +pytest_test( + name = "sync_changelog_test", + srcs = ["sync_changelog_test.py"], + python_version = "3.14", + target_compatible_with = NOT_WINDOWS, + deps = [ + ":conftest", + ":release_test_helper", + "//tools/private/release:release_lib", + ], +) + pytest_test( name = "create_release_branch_test", srcs = ["create_release_branch_test.py"], diff --git a/tests/tools/private/release/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index dd768400ba..1acc93a192 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -1,9 +1,8 @@ import argparse import datetime import logging -from unittest.mock import ANY, call +from unittest.mock import call -from tools.private.release.gh import CreatePrError from tools.private.release.process_backports import ProcessBackports pytest_plugins = ["tests.tools.private.release.release_test_helper"] @@ -58,11 +57,8 @@ def test_process_backports_success(mocker, mock_git, mock_gh): } mock_git.get_remote_tags.return_value = [] mock_git.sort_commits_chronologically.return_value = ["abcdef12"] - mock_git.get_commit_sha.side_effect = ["12345678", "12345678", "main_sha"] + mock_git.get_commit_sha.side_effect = ["12345678", "12345678"] mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' - mock_git.get_modified_files.return_value = ["news/124.fixed.md"] - mock_git.diff.return_value = "version diff for 124" - mock_git.apply_check.return_value = True result = ProcessBackports(args, mock_git, mock_gh).run() @@ -71,94 +67,22 @@ def test_process_backports_success(mocker, mock_git, mock_gh): [ call("origin", tags=True, force=True), call("origin"), - call("origin", refspec="main"), - ] - ) - mock_git.checkout.assert_has_calls( - [ - call("release/2.0", track_remote="origin"), - call("main", track_remote="origin"), - call("prepare-2.0.0-backports-6affdae", create_branch=True), - call("release/2.0"), ] ) + mock_git.checkout.assert_called_once_with("release/2.0", track_remote="origin") mock_git.cherry_pick.assert_called_once_with("abcdef12") - mock_git.diff.assert_called_once() - mock_git.apply_check.assert_called_once_with(ANY) - mock_git.apply.assert_called_once_with(ANY) - mock_changelog.update_changelog.assert_has_calls( - [ - call("2.0.0", "2026-07-01"), - call( - "2.0.0", - "2026-07-01", - news_files=["news/124.fixed.md"], - delete_news=True, - ), - ] - ) - assert mock_git.add_modified_and_deleted.call_count == 2 + mock_changelog.update_changelog.assert_called_once_with("2.0.0", "2026-07-01") + mock_git.add_modified_and_deleted.assert_called_once() mock_replace.assert_called_once_with("2.0.0") - mock_git.commit.assert_has_calls( - [ - call('Cherry-pick "fix bug"\n\nWork towards #123', amend=True), - call("chore(release): sync changelog for v2.0.0 backports"), - ] + mock_git.commit.assert_called_once_with( + 'Cherry-pick "fix bug"\n\nWork towards #123', amend=True ) + mock_git.push.assert_called_once_with("origin", "release/2.0") updated_body = mock_gh.get_issue_body(123) assert "- [x] #124 | status=done rc=rc0 commit= 12345678" in updated_body - assert "- [ ] Sync Changelog #124 | status=pending pr=#1001" in updated_body - - -def test_process_backports_sync_branch_exists(mocker, mock_git, mock_gh): - mocker.patch("tools.private.release.process_backports.changelog_news") - mocker.patch("tools.private.release.process_backports.replace_version_next") - mock_datetime = mocker.patch("tools.private.release.process_backports.datetime") - mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) - - args = argparse.Namespace( - issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None - ) - mock_gh.issues[123] = { - "title": "Release 2.0.0", - "body": """ -## Checklist -- [ ] Prepare Release -- [ ] Create Release branch -- [ ] Sync Changelog #124 -- [ ] Tag Final - -## Backports -- [ ] #124 | status=pending -""", - "labels": ["type: release"], - } - mock_gh.prs[124] = { - "state": "MERGED", - "mergeCommit": {"oid": "abcdef12"}, - } - mock_git.get_remote_tags.return_value = [] - mock_git.sort_commits_chronologically.return_value = ["abcdef12"] - mock_git.get_commit_sha.side_effect = ["12345678", "12345678", "main_sha"] - mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' - mock_git.get_modified_files.return_value = ["news/124.fixed.md"] - mock_git.diff.return_value = "version diff for 124" - mock_git.apply_check.return_value = True - mock_git.branch_exists.return_value = True - - result = ProcessBackports(args, mock_git, mock_gh).run() - - assert result == 0 - mock_git.checkout.assert_has_calls( - [ - call("release/2.0", track_remote="origin"), - call("main", track_remote="origin"), - call("prepare-2.0.0-backports-6affdae"), - call("release/2.0"), - ] - ) - mock_git.reset_hard.assert_has_calls([call(reset_to="main")]) + # Sync Changelog task remains untouched by process_backports + assert "- [ ] Sync Changelog #124" in updated_body def test_process_backports_dry_run(mocker, mock_git, mock_gh): @@ -190,16 +114,12 @@ def test_process_backports_dry_run(mocker, mock_git, mock_gh): } mock_git.get_remote_tags.return_value = [] mock_git.sort_commits_chronologically.return_value = ["abcdef12"] - mock_git.get_commit_sha.side_effect = ["12345678", "main_sha"] + mock_git.get_commit_sha.return_value = "12345678" mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' - mock_git.get_modified_files.return_value = ["news/124.fixed.md"] - mock_git.diff.return_value = "version diff for 124" - mock_git.apply_check.return_value = True result = ProcessBackports(args, mock_git, mock_gh).run() assert result == 0 - mock_git.apply.assert_not_called() mock_git.push.assert_not_called() @@ -335,15 +255,13 @@ def test_process_backports_add_backports_and_auto_add_rc_task( mock_git.get_commit_sha.return_value = "12345678" mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' mock_git.sort_commits_chronologically.return_value = ["abcdef12"] - mock_git.diff.return_value = "version diff" - mock_git.apply_check.return_value = True result = ProcessBackports(args, mock_git, mock_gh).run() assert result == 0 updated_body = mock_gh.get_issue_body(123) assert "- [x] #124 | status=done rc=rc1 commit= 12345678" in updated_body - assert "- [ ] Sync Changelog #124 | status=pending pr=#1001" in updated_body + assert "- [ ] Sync Changelog #124" in updated_body def test_process_backports_add_backports_marks_invalid(mocker, mock_git, mock_gh): @@ -383,180 +301,14 @@ def test_process_backports_add_backports_marks_invalid(mocker, mock_git, mock_gh mock_git.get_commit_sha.return_value = "1234567890" mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' mock_git.sort_commits_chronologically.return_value = ["sha_124", "sha_125"] - mock_git.diff.return_value = "version diff" - mock_git.apply_check.return_value = True result = ProcessBackports(args, mock_git, mock_gh).run() assert result == 0 updated_body = mock_gh.get_issue_body(123) assert "- [ ] invalid | status=error-invalid-pr" in updated_body - assert "- [ ] Sync Changelog #124 | status=pending pr=#1001" in updated_body - assert "- [ ] Sync Changelog #125 | status=pending pr=#1001" in updated_body - - -def test_process_backports_version_sync_failure(mocker, mock_git, mock_gh): - mock_changelog = mocker.patch( - "tools.private.release.process_backports.changelog_news" - ) - mock_replace = mocker.patch( - "tools.private.release.process_backports.replace_version_next" - ) - mock_datetime = mocker.patch("tools.private.release.process_backports.datetime") - mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) - args = argparse.Namespace( - issue=123, remote="origin", dry_run=False, add=None, triggering_comment=None - ) - mock_gh.issues[123] = { - "title": "Release 2.0.0", - "body": """ -## Checklist -- [ ] Prepare Release -- [ ] Create Release branch -- [ ] Sync Changelog #124 -- [ ] Sync Changelog #125 -- [ ] Tag Final - -## Backports -- [ ] #124 | status=pending -- [ ] #125 | status=pending -""", - "labels": ["type: release"], - } - mock_gh.prs[124] = { - "state": "MERGED", - "mergeCommit": {"oid": "sha_124"}, - } - mock_gh.prs[125] = { - "state": "MERGED", - "mergeCommit": {"oid": "sha_125"}, - } - mock_git.get_remote_tags.return_value = [] - mock_git.sort_commits_chronologically.return_value = ["sha_124", "sha_125"] - mock_git.get_commit_sha.side_effect = [ - "12345678", - "sha_124_amended", - "sha_125_amended", - "main_sha", - ] - mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' - mock_git.get_modified_files.side_effect = [ - ["news/124.fixed.md"], - ["news/125.fixed.md"], - ] - mock_git.diff.return_value = "diff content" - mock_git.apply_check.side_effect = [False, True] - - result = ProcessBackports(args, mock_git, mock_gh).run() - - assert result == 0 - mock_git.checkout.assert_has_calls( - [ - call("release/2.0", track_remote="origin"), - call("main", track_remote="origin"), - call("prepare-2.0.0-backports-b552a96", create_branch=True), - call("release/2.0"), - ] - ) - mock_git.cherry_pick.assert_has_calls( - [ - call("sha_124"), - call("sha_125"), - ] - ) - assert mock_git.diff.call_count == 2 - assert mock_git.apply_check.call_count == 2 - mock_git.apply.assert_called_once_with(ANY) - - mock_changelog.update_changelog.assert_has_calls( - [ - call("2.0.0", "2026-07-01"), - call("2.0.0", "2026-07-01"), - call( - "2.0.0", - "2026-07-01", - news_files=["news/124.fixed.md", "news/125.fixed.md"], - delete_news=True, - ), - ] - ) - assert mock_git.add_modified_and_deleted.call_count == 3 - assert mock_replace.call_count == 2 - - assert 1001 in mock_gh.prs - pr = mock_gh.prs[1001] - assert ( - "Warning: These PRs failed to update their version markers:\n- #124" - in pr["body"] - ) - - updated_body = mock_gh.get_issue_body(123) - assert "- [ ] Sync Changelog #124 | status=pending pr=#1001" in updated_body - assert "- [ ] Sync Changelog #125 | status=pending pr=#1001" in updated_body - - -def test_process_backports_sync_changelog_create_pr_failure( - mocker, mock_git, mock_gh, capsys, caplog -): - mocker.patch("tools.private.release.process_backports.changelog_news") - mocker.patch("tools.private.release.process_backports.replace_version_next") - mock_datetime = mocker.patch("tools.private.release.process_backports.datetime") - mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) - - args = argparse.Namespace( - issue=123, - remote="origin", - dry_run=False, - add=None, - triggering_comment=5297050431, - ) - mock_gh.issues[123] = { - "title": "Release 2.0.0", - "body": """ -## Checklist -- [ ] Prepare Release -- [ ] Create Release branch -- [ ] Sync Changelog #124 -- [ ] Tag Final - -## Backports -- [ ] #124 | status=pending -""", - "labels": ["type: release"], - } - mock_gh.prs[124] = { - "state": "MERGED", - "mergeCommit": {"oid": "abcdef12"}, - } - mock_git.get_remote_tags.return_value = [] - mock_git.sort_commits_chronologically.return_value = ["abcdef12"] - mock_git.get_commit_sha.side_effect = ["12345678", "12345678", "main_sha"] - mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' - mock_git.get_modified_files.return_value = ["news/124.fixed.md"] - mock_git.diff.return_value = "version diff for 124" - mock_git.apply_check.return_value = True - - # Make create_pr raise CreatePrError - err = CreatePrError( - "Failed to create PR 'chore(release): sync changelog for v2.0.0 backports': " - "Command '['gh', 'pr', 'create']' returned non-zero exit status 1.\n" - "Error running command: gh pr create ...\nStdout: \nStderr: pull request already exists" - ) - mocker.patch.object(mock_gh, "create_pr", side_effect=err) - - with caplog.at_level(logging.ERROR): - result = ProcessBackports(args, mock_git, mock_gh).run() - - assert result == 1 - assert mock_gh.reactions.get(5297050431) == ["-1"] - - captured = capsys.readouterr() - assert ( - "Unexpected error: Failed to create PR 'chore(release): sync changelog for v2.0.0 backports'" - in caplog.text - ) - assert "Error running command: gh pr create ..." in captured.err - assert "Stderr: pull request already exists" in captured.err + assert "- [ ] Sync Changelog #124" in updated_body + assert "- [ ] Sync Changelog #125" in updated_body def test_process_backports_logs_no_pending(mock_git, mock_gh, caplog): diff --git a/tests/tools/private/release/sync_changelog_test.py b/tests/tools/private/release/sync_changelog_test.py new file mode 100644 index 0000000000..4de642e9ff --- /dev/null +++ b/tests/tools/private/release/sync_changelog_test.py @@ -0,0 +1,361 @@ +import argparse +from unittest.mock import MagicMock, call + +from tools.private.release.gh import CreatePrError +from tools.private.release.sync_changelog import SyncChangelog + +pytest_plugins = ["tests.tools.private.release.release_test_helper"] + + +def test_sync_changelog_no_pending(mock_git, mock_gh): + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [x] Sync Changelog #124 | status=done +""", + "labels": ["type: release"], + } + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + mock_git.fetch.assert_not_called() + mock_git.checkout.assert_not_called() + + +def test_sync_changelog_success(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Tag Final + +## Backports +- [x] #124 | status=done rc=rc0 commit= abcdef12 +""", + "labels": ["type: release"], + } + mock_git.branch_exists.return_value = False + mock_git.get_commit_sha.return_value = "main_sha" + # Git status returns clean on initial check, dirty after process_news + mock_git.status.side_effect = ["", "M CHANGELOG.md\nD news/124.fixed.md"] + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + mock_git.fetch.assert_called_once_with("origin", refspec="main") + mock_git.checkout.assert_has_calls( + [ + call("main", track_remote="origin"), + call("prepare-2.0.0-backports-6affdae", create_branch=True), + call("main"), + ] + ) + mock_process_news_class.assert_called_once() + assert mock_process_news_class.call_args[0][0].version == "2.0.0" + assert mock_process_news_class.call_args[0][0].targets == ["124"] + + mock_git.add_modified_and_deleted.assert_called_once() + mock_git.commit.assert_called_once_with( + "chore(release): sync changelog for v2.0.0 backports" + ) + mock_git.push.assert_called_once_with( + "origin", + "prepare-2.0.0-backports-6affdae", + set_upstream=True, + force=True, + ) + + updated_body = mock_gh.get_issue_body(123) + assert "- [ ] Sync Changelog #124 | status=pending pr=#1001" in updated_body + + +def test_sync_changelog_branch_exists(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +""", + "labels": ["type: release"], + } + mock_git.branch_exists.return_value = True + mock_git.get_commit_sha.return_value = "main_sha" + mock_git.status.side_effect = ["", "M CHANGELOG.md"] + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + mock_git.checkout.assert_has_calls( + [ + call("main", track_remote="origin"), + call("prepare-2.0.0-backports-6affdae"), + call("main"), + ] + ) + mock_git.reset_hard.assert_called_once_with(reset_to="main") + + +def test_sync_changelog_dry_run(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=True, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +""", + "labels": ["type: release"], + } + mock_git.get_commit_sha.return_value = "main_start_sha" + mock_git.status.side_effect = ["", "M CHANGELOG.md", "M CHANGELOG.md"] + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + mock_git.push.assert_not_called() + mock_git.commit.assert_not_called() + mock_git.reset_hard.assert_called_once_with(reset_to="main_start_sha") + + +def test_sync_changelog_auto_discover_issue(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=None, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "number": 123, + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +""", + "labels": ["type: release"], + } + mock_git.status.side_effect = ["", "M CHANGELOG.md"] + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + assert ( + "- [ ] Sync Changelog #124 | status=pending pr=#1001" + in mock_gh.get_issue_body(123) + ) + + +def test_sync_changelog_multiple_open_issues_fails(mock_git, mock_gh): + args = argparse.Namespace( + issue=None, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "number": 123, + "title": "Release 2.0.0", + "body": "", + "labels": ["type: release"], + } + mock_gh.issues[124] = { + "number": 124, + "title": "Release 2.1.0", + "body": "", + "labels": ["type: release"], + } + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 1 + mock_git.fetch.assert_not_called() + + +def test_sync_changelog_specific_prs_arg(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + prs=["#124", "125"], + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +- [ ] Sync Changelog #125 +""", + "labels": ["type: release"], + } + mock_git.status.side_effect = ["", "M CHANGELOG.md"] + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + mock_process_news_class.assert_called_once() + assert mock_process_news_class.call_args[0][0].targets == ["124", "125"] + + +def test_sync_changelog_no_changes(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +""", + "labels": ["type: release"], + } + # No changes after running process news + mock_git.status.side_effect = ["", ""] + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + mock_git.commit.assert_not_called() + mock_git.push.assert_not_called() + + +def test_sync_changelog_process_news_failure(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 1 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=55555, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +""", + "labels": ["type: release"], + } + mock_git.status.return_value = "" + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 1 + assert mock_gh.reactions.get(55555) == ["-1"] + + +def test_sync_changelog_create_pr_failure(mocker, mock_git, mock_gh): + mock_process_news_class = mocker.patch( + "tools.private.release.sync_changelog.ProcessNews" + ) + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + prs=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +""", + "labels": ["type: release"], + } + mock_git.status.side_effect = ["", "M CHANGELOG.md"] + + err = CreatePrError("Failed to create PR") + mocker.patch.object(mock_gh, "create_pr", side_effect=err) + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 1 diff --git a/tests/workflows/on_comment_test.py b/tests/workflows/on_comment_test.py index d7df7da13e..8ec6780456 100644 --- a/tests/workflows/on_comment_test.py +++ b/tests/workflows/on_comment_test.py @@ -202,6 +202,19 @@ def test_release_issue_backport_empty(monkeypatch, gha_env, mock_add_reaction, c ) +def test_release_issue_sync_changelog(monkeypatch, gha_env): + _run_comment( + monkeypatch, + "/sync-changelog", + has_release_label="true", + ) + assert gha_env.read_outputs() == { + "issue_number": "100", + "command": "sync-changelog", + } + assert gha_env.read_env() == {"issue_number": "100"} + + def test_release_issue_promote(monkeypatch, gha_env): _run_comment( monkeypatch, diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index ce1d54ee34..50c6aadbe3 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -2,10 +2,7 @@ import argparse import datetime -import hashlib import logging -import os -import tempfile import traceback from dataclasses import dataclass from typing import Any @@ -36,12 +33,6 @@ class CherryPickAndUpdatePrsResult: # List of PR references that failed to cherry-pick. failed_prs: list[str] - # List of news files collected from the successful cherry-picks. - collected_news_files: list[str] - # List of PR numbers that were successfully cherry-picked. - successful_pr_nums: list[int] - # List of tuples mapping successful PR numbers to their version marker diffs. - collected_diffs: list[tuple[int, str]] # The updated checklist body for the release tracking issue. body: str @@ -113,28 +104,16 @@ def _cherry_pick_and_update_prs( next_rc_suffix, ) -> CherryPickAndUpdatePrsResult: failed_prs = [] - collected_news_files = [] - successful_pr_nums = [] - collected_diffs = [] for sha in sorted_shas: item = sha_to_item[sha] logger.info("Cherry-picking %s / %s...", item.pr_ref, sha) try: self.git.cherry_pick(sha) - # Collect news files before they are deleted by update_changelog - modified_files = self.git.get_modified_files("HEAD") - for f in modified_files: - if changelog_news.is_news_file(f): - collected_news_files.append(f) - - # Replace version markers FIRST to isolate diff + # Replace version markers FIRST logger.info("Replacing version markers for PR %s...", item.pr_ref) replace_version_next(version) - # Get diff of unstaged changes (version marker replacement) - diff_content = self.git.diff() - # Perform news processing (merging news/ files into the changelog) logger.info( "Merging news fragments into changelog for PR %s...", @@ -153,18 +132,6 @@ def _cherry_pick_and_update_prs( new_msg = f"{current_msg.strip()}\n\nWork towards #{issue}" self.git.commit(new_msg, amend=True) - try: - pr_num = self.gh.resolve_pr_number(item.pr_ref) - if diff_content: - collected_diffs.append((pr_num, diff_content)) - successful_pr_nums.append(pr_num) - except Exception as e: - logger.warning( - "Failed to resolve PR number for %s: %s", - item.pr_ref, - format_exception(e), - ) - if not dry_run: # Push amended commit self.git.push(remote, branch_name) @@ -251,213 +218,9 @@ def _cherry_pick_and_update_prs( ) return CherryPickAndUpdatePrsResult( failed_prs=failed_prs, - collected_news_files=collected_news_files, - successful_pr_nums=successful_pr_nums, - collected_diffs=collected_diffs, body=body, ) - def _sync_changelog_to_main( - self, - version: str, - collected_news_files: list[str], - successful_pr_nums: list[int], - collected_diffs: list[tuple[int, str]], - release_branch: str, - ) -> None: - args = self.args - sorted_prs = sorted(successful_pr_nums) - prs_str = ",".join(str(n) for n in sorted_prs) - prs_hash = hashlib.sha256(prs_str.encode()).hexdigest()[:7] - - main_branch = "main" - backport_branch = f"prepare-{version}-backports-{prs_hash}" - - logger.info( - "Syncing changelog to %s via branch %s...", - main_branch, - backport_branch, - ) - - self.git.fetch(args.remote, refspec=main_branch) - self.git.checkout(main_branch, track_remote=args.remote) - main_start_sha = self.git.get_commit_sha("HEAD") - - failed_version_sync_prs = [] - try: - if args.dry_run: - logger.info( - "[DRY RUN] Would create and checkout branch %s from %s", - backport_branch, - main_branch, - ) - else: - if self.git.branch_exists(backport_branch): - self.git.checkout(backport_branch) - self.git.reset_hard(reset_to=main_branch) - else: - self.git.checkout(backport_branch, create_branch=True) - - logger.info( - "Updating CHANGELOG.md and removing news files on %s...", - backport_branch, - ) - release_date = datetime.date.today().strftime("%Y-%m-%d") - changelog_news.update_changelog( - version, - release_date, - news_files=collected_news_files, - delete_news=True, - ) - - # Apply version marker diffs - failed_version_sync_prs = self._apply_version_marker_diffs(collected_diffs) - - if args.dry_run: - logger.info( - "[DRY RUN] Would commit: 'chore(release): sync changelog" - " for v%s backports'", - version, - ) - logger.info( - "[DRY RUN] Would push %s to %s", - backport_branch, - args.remote, - ) - logger.info( - "[DRY RUN] Would create PR to %s with label 'type: sync-changelog'", - main_branch, - ) - logger.info( - "[DRY RUN] Would update tracking issue #%s checklist tasks" - " 'Sync Changelog #' to PENDING", - args.issue, - ) - logger.info("[DRY RUN] Diff of changes:\n%s", self.git.status()) - else: - self.git.add_modified_and_deleted() - self.git.commit( - f"chore(release): sync changelog for v{version} backports" - ) - self.git.push( - args.remote, backport_branch, set_upstream=True, force=True - ) - - pr_title = f"chore(release): sync changelog for v{version} backports" - pr_body_lines = [ - "Updates CHANGELOG.md and removes news files for backports:", - ] - for pr_num in sorted_prs: - pr_body_lines.append(f"- #{pr_num}") - - if failed_version_sync_prs: - pr_body_lines.append("") - pr_body_lines.append( - "Warning: These PRs failed to update their version markers:" - ) - for pr_num in sorted(failed_version_sync_prs): - pr_body_lines.append(f"- #{pr_num}") - - pr_body_lines.append("") - pr_body_lines.append(f"Work towards #{args.issue}") - pr_body_lines.append(f"Release-Tracking-Issue: #{args.issue}") - pr_body = "\n".join(pr_body_lines) - - logger.info("Creating PR to %s...", main_branch) - pr_url = self.gh.create_pr( - title=pr_title, - body=pr_body, - base=main_branch, - labels=["type: sync-changelog"], - ) - logger.info("Created PR: %s", pr_url) - - try: - pr_num = int(pr_url.split("/")[-1]) - logger.info("Enabling auto-merge for PR #%s...", pr_num) - self.gh.enable_auto_merge(pr_num) - - logger.info( - "Updating tracking issue #%s checklist with" - " Sync Changelog tasks...", - args.issue, - ) - issue_body = self.gh.get_issue_body(args.issue) - for pr in successful_pr_nums: - task_name = f"Sync Changelog #{pr}" - metadata = {"status": "pending", "pr": f"#{pr_num}"} - issue_body = update_task_in_body( - issue_body, - task_name, - checked=False, - metadata=metadata, - ) - self.gh.update_issue_body(args.issue, issue_body) - except Exception as e: - logger.warning( - "Failed to update tracking issue or enable auto-merge: %s", - format_exception(e), - ) - finally: - if args.dry_run: - logger.info( - "[DRY RUN] Resetting branch %s to %s after changelog sync dry run", - main_branch, - main_start_sha, - ) - self.git.reset_hard(reset_to=main_start_sha) - logger.info( - "Restoring checkout of release branch %s after syncing changelog to main", - release_branch, - ) - self.git.checkout(release_branch) - - def _apply_version_marker_diffs( - self, - collected_diffs: list[tuple[int, str]], - ) -> list[int]: - """Applies version marker diffs on main branch and returns failed PR numbers.""" - args = self.args - failed_version_sync_prs = [] - if not collected_diffs: - return failed_version_sync_prs - - with tempfile.TemporaryDirectory() as temp_dir: - logger.info("Applying %d version marker patches...", len(collected_diffs)) - for pr_num, diff_content in collected_diffs: - if args.dry_run: - logger.info( - "[DRY RUN] Would check and apply version marker patch" - " for PR #%s", - pr_num, - ) - - patch_filepath = os.path.join(temp_dir, f"{pr_num}.patch") - with open(patch_filepath, "w", encoding="utf-8") as f: - f.write(diff_content) - - if self.git.apply_check(patch_filepath): - if args.dry_run: - logger.info( - "[DRY RUN] Version marker patch for PR #%s applies" - " cleanly.", - pr_num, - ) - else: - logger.info( - "Applying version marker patch for PR #%s...", - pr_num, - ) - self.git.apply(patch_filepath) - else: - logger.warning( - "Version marker patch for PR #%s could not be applied" - " cleanly to main. Skipping.", - pr_num, - ) - failed_version_sync_prs.append(pr_num) - return failed_version_sync_prs - def run(self) -> int: """Executes the process-backports subcommand.""" args = self.args @@ -620,9 +383,6 @@ def _run_internal(self) -> int: self.git.checkout(branch_name, track_remote=args.remote) start_sha = self.git.get_commit_sha("HEAD") - collected_news_files = [] - successful_pr_nums = [] - collected_diffs = [] try: result = self._cherry_pick_and_update_prs( sorted_shas, @@ -636,9 +396,6 @@ def _run_internal(self) -> int: next_rc_suffix, ) failed_prs.extend(result.failed_prs) - collected_news_files.extend(result.collected_news_files) - successful_pr_nums.extend(result.successful_pr_nums) - collected_diffs.extend(result.collected_diffs) body = result.body finally: if args.dry_run: @@ -649,15 +406,6 @@ def _run_internal(self) -> int: ) self.git.reset_hard(reset_to=start_sha) - if successful_pr_nums: - self._sync_changelog_to_main( - version, - collected_news_files, - successful_pr_nums, - collected_diffs, - branch_name, - ) - if failed_prs: logger.error("One or more cherry-picks/resolutions failed:") for pr in failed_prs: diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 9df5ba8606..51c18a3d1d 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -19,6 +19,7 @@ from tools.private.release.process_backports import ProcessBackports from tools.private.release.process_news import ProcessNews from tools.private.release.promote import Promote +from tools.private.release.sync_changelog import SyncChangelog from tools.private.release.utils import format_exception cmds = [ @@ -31,6 +32,7 @@ AddBackports, ProcessBackports, ProcessNews, + SyncChangelog, OnPrMerged, CreateRc, Promote, diff --git a/tools/private/release/sync_changelog.py b/tools/private/release/sync_changelog.py new file mode 100644 index 0000000000..19a5339dae --- /dev/null +++ b/tools/private/release/sync_changelog.py @@ -0,0 +1,303 @@ +"""Subcommand to create sync PR to main for backports in a release.""" + +import argparse +import hashlib +import logging +import traceback + +from tools.private.release.gh import ( + GH_REACTION_THUMBS_DOWN, + GitHub, + GitHubInterface, +) +from tools.private.release.git import Git +from tools.private.release.process_news import ProcessNews +from tools.private.release.release_issue import ( + RELEASE_TITLE_RE, + parse_checklist_state, + update_task_in_body, +) +from tools.private.release.utils import format_exception, parse_pr_list + +logger = logging.getLogger(__name__) + + +class SyncChangelog: + """Class to sync changelog to main for backports in a release.""" + + def __init__(self, args, git: Git, gh: GitHubInterface): + self.args = args + self.git = git + self.gh = gh + + def run(self) -> int: + """Executes the sync-changelog subcommand.""" + args = self.args + exit_code = 0 + try: + exit_code = self._run_internal() + except Exception as e: + logger.error("Unexpected error: %s", e) + traceback.print_exc() + exit_code = 1 + + if exit_code != 0 and args.triggering_comment: + logger.info( + "Reacting with thumbs-down to comment %s...", + args.triggering_comment, + ) + try: + self.gh.add_comment_reaction( + args.triggering_comment, GH_REACTION_THUMBS_DOWN + ) + except Exception as e: + logger.error("Failed to add reaction to comment: %s", e) + + return exit_code + + def _run_internal(self) -> int: + """Internal implementation of sync-changelog.""" + args = self.args + issue_num = args.issue + if not issue_num: + logger.info( + "No issue specified. Auto-discovering open release tracking issue..." + ) + open_issues = self.gh.get_open_tracking_issues() + if len(open_issues) > 1: + logger.error( + "Multiple open release tracking issues found: %s", + [f"#{i['number']}" for i in open_issues], + ) + return 1 + elif len(open_issues) == 1: + issue_num = open_issues[0]["number"] + logger.info("Discovered release tracking issue #%d", issue_num) + else: + logger.error("No open release tracking issues found.") + return 1 + + body = self.gh.get_issue_body(issue_num) + issue_title = self.gh.get_issue_title(issue_num) + version_match = RELEASE_TITLE_RE.search(issue_title) + if not version_match: + logger.error("Could not parse version from issue title: %s", issue_title) + return 1 + + version = version_match.group(1) + + if args.prs: + pending_prs = [] + for pr_ref in args.prs: + try: + pr_num = self.gh.resolve_pr_number(pr_ref) + pending_prs.append(pr_num) + except Exception as e: + logger.error( + "Failed to resolve PR reference '%s': %s", + pr_ref, + format_exception(e), + ) + return 1 + else: + state = parse_checklist_state(body) + sync_tasks = state.get("sync_changelogs", {}) + pending_prs = [ + pr_num + for pr_num, task in sync_tasks.items() + if not task.checked + and task.status != "done" + and not (task.status or "").startswith("error-") + ] + + if not pending_prs: + logger.info("No pending sync changelog tasks found.") + return 0 + + logger.info( + "Found %d pending sync changelog tasks to process: %s", + len(pending_prs), + pending_prs, + ) + + if self.git.status(): + logger.error( + "Git workspace is dirty. Please commit or stash changes" + " before running sync-changelog." + ) + return 1 + + sorted_prs = sorted(pending_prs) + prs_str = ",".join(str(n) for n in sorted_prs) + prs_hash = hashlib.sha256(prs_str.encode()).hexdigest()[:7] + + main_branch = "main" + backport_branch = f"prepare-{version}-backports-{prs_hash}" + + self.git.fetch(args.remote, refspec=main_branch) + self.git.checkout(main_branch, track_remote=args.remote) + main_start_sha = self.git.get_commit_sha("HEAD") + + try: + if args.dry_run: + logger.info( + "[DRY RUN] Would create and checkout branch %s from %s", + backport_branch, + main_branch, + ) + else: + if self.git.branch_exists(backport_branch): + self.git.checkout(backport_branch) + self.git.reset_hard(reset_to=main_branch) + else: + self.git.checkout(backport_branch, create_branch=True) + + # Run ProcessNews to process news files and version markers + process_news_args = argparse.Namespace( + version=version, + targets=[str(pr) for pr in sorted_prs], + ) + process_news_runner = ProcessNews(process_news_args, gh=self.gh) + ret = process_news_runner.run() + if ret != 0: + logger.error("ProcessNews failed for targets: %s", sorted_prs) + return 1 + + if not self.git.status(): + logger.info("No changes to sync after running process-news.") + return 0 + + if args.dry_run: + logger.info( + "[DRY RUN] Would commit: 'chore(release): sync changelog" + " for v%s backports'", + version, + ) + logger.info( + "[DRY RUN] Would push %s to %s", + backport_branch, + args.remote, + ) + logger.info( + "[DRY RUN] Would create PR to %s with label 'type: sync-changelog'", + main_branch, + ) + logger.info( + "[DRY RUN] Would update tracking issue #%s checklist tasks" + " 'Sync Changelog #' to PENDING", + issue_num, + ) + logger.info("[DRY RUN] Diff of changes:\n%s", self.git.status()) + else: + self.git.add_modified_and_deleted() + self.git.commit( + f"chore(release): sync changelog for v{version} backports" + ) + self.git.push( + args.remote, backport_branch, set_upstream=True, force=True + ) + + pr_title = f"chore(release): sync changelog for v{version} backports" + pr_body_lines = [ + "Updates CHANGELOG.md and removes news files for backports:", + ] + for pr_num in sorted_prs: + pr_body_lines.append(f"- #{pr_num}") + + pr_body_lines.append("") + pr_body_lines.append(f"Work towards #{issue_num}") + pr_body_lines.append(f"Release-Tracking-Issue: #{issue_num}") + pr_body = "\n".join(pr_body_lines) + + logger.info("Creating PR to %s...", main_branch) + pr_url = self.gh.create_pr( + title=pr_title, + body=pr_body, + base=main_branch, + labels=["type: sync-changelog"], + ) + logger.info("Created PR: %s", pr_url) + + try: + pr_num = int(pr_url.split("/")[-1]) + logger.info("Enabling auto-merge for PR #%s...", pr_num) + self.gh.enable_auto_merge(pr_num) + + logger.info( + "Updating tracking issue #%s checklist with" + " Sync Changelog tasks...", + issue_num, + ) + issue_body = self.gh.get_issue_body(issue_num) + for pr in sorted_prs: + task_name = f"Sync Changelog #{pr}" + metadata = {"status": "pending", "pr": f"#{pr_num}"} + issue_body = update_task_in_body( + issue_body, + task_name, + checked=False, + metadata=metadata, + ) + self.gh.update_issue_body(issue_num, issue_body) + except Exception as e: + logger.warning( + "Failed to update tracking issue or enable auto-merge: %s", + format_exception(e), + ) + finally: + if args.dry_run: + logger.info( + "[DRY RUN] Resetting branch %s to %s after changelog sync dry run", + main_branch, + main_start_sha, + ) + self.git.reset_hard(reset_to=main_start_sha) + self.git.checkout(main_branch) + + return 0 + + @classmethod + def add_parser(cls, subparsers): + """Adds parser for sync-changelog subcommand.""" + parser = subparsers.add_parser( + "sync-changelog", + help="Create a sync PR to main for backports in a release.", + ) + parser.add_argument( + "--issue", + type=int, + help="The tracking issue number (optional; auto-discovered if omitted).", + ) + parser.add_argument( + "--remote", + type=str, + required=True, + help="The git remote to push changes to (required).", + ) + parser.add_argument( + "--prs", + type=parse_pr_list, + help=( + "PR references (numbers, #numbers, or URLs, comma/space" + " separated) to sync (optional)." + ), + ) + parser.add_argument( + "--triggering-comment", + type=int, + help="The ID of the comment that triggered this run (optional).", + ) + parser.add_argument( + "--dry-run", + action=argparse.BooleanOptionalAction, + default=True, + help="Perform a dry run (default: True). Use --no-dry-run to actually execute.", + ) + parser.set_defaults(command=cls.run_from_args) + + @classmethod + def run_from_args(cls, args): + """Instantiates and runs the command from parsed args.""" + git = Git(".") + gh = GitHub() + return cls(args, git, gh).run() From 6a8d1930c584d0de8fe9707f9eaa24b15f11154e Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 17 Aug 2026 06:41:29 +0000 Subject: [PATCH 2/5] refactor(release): separate sync changelog workflows and improve event input handling Refactor sync changelog processing into dedicated workflows and streamline event input resolution. Why: - Isolate sync changelog and completion jobs into separate reusable workflows for clarity and maintainability. - Avoid passing manual CLI arguments in GitHub Actions by reading event context directly from GITHUB_EVENT_PATH. - Improve logging visibility with workflow command annotations and clean issue comments. How: - Extract complete sync changelog logic into release_sync_changelog_complete.yaml workflow. - Update release_sync_changelog.yaml and release_sync_changelog_complete.yaml to run without CLI flag passing. - Add GitHubEventDict and GITHUB_EVENT_PATH parser helpers to gh.py. - Configure GitHubActionsLogHandler in release.py for GHA notices, warnings, and errors. - Simplify tracking issue comment messages and link to workflow runs. --- .github/workflows/on_comment.py | 2 +- .github/workflows/on_comment.yaml | 1 - .github/workflows/on_pr_closed.yaml | 60 +--- .../workflows/release_process_backports.yaml | 2 +- .github/workflows/release_sync_changelog.yaml | 23 +- .../release_sync_changelog_complete.yaml | 45 +++ RELEASING.md | 4 +- .../release/complete_sync_changelog_test.py | 44 +++ .../private/release/sync_changelog_test.py | 85 +++--- .../release/complete_sync_changelog.py | 48 ++-- tools/private/release/gh.py | 91 +++++- tools/private/release/release.py | 20 ++ tools/private/release/sync_changelog.py | 267 +++++++++--------- 13 files changed, 420 insertions(+), 272 deletions(-) create mode 100644 .github/workflows/release_sync_changelog_complete.yaml diff --git a/.github/workflows/on_comment.py b/.github/workflows/on_comment.py index 2672db3cd1..7a0ac45f26 100755 --- a/.github/workflows/on_comment.py +++ b/.github/workflows/on_comment.py @@ -101,7 +101,7 @@ def _process_release_issue_comment( _write_github_output("command", "process-backports") return - if _match_command(("sync-changelog", "sync_changelog"), comment_body): + if _match_command("sync-changelog", comment_body): _write_github_output("command", "sync-changelog") return diff --git a/.github/workflows/on_comment.yaml b/.github/workflows/on_comment.yaml index 4620fe8ccf..487d114a11 100644 --- a/.github/workflows/on_comment.yaml +++ b/.github/workflows/on_comment.yaml @@ -114,7 +114,6 @@ jobs: uses: ./.github/workflows/release_sync_changelog.yaml with: issue: ${{ needs.parse_comment.outputs.issue_number }} - comment_id: "${{ github.event.comment.id }}" secrets: inherit call_promote: diff --git a/.github/workflows/on_pr_closed.yaml b/.github/workflows/on_pr_closed.yaml index e21864769e..1c2b273d04 100644 --- a/.github/workflows/on_pr_closed.yaml +++ b/.github/workflows/on_pr_closed.yaml @@ -82,64 +82,16 @@ jobs: --remote origin \ --no-dry-run - sync_changelog: + call_sync_changelog: needs: process_backports - runs-on: ubuntu-latest - permissions: - contents: write - issues: write - pull-requests: write - steps: - - name: Checkout repository - uses: actions/checkout@v7 - with: - fetch-depth: 0 - - - name: Setup Bazel - uses: bazel-contrib/setup-bazel@0.19.0 - with: - bazelisk-version: 1.20.0 - - - name: Configure Git Identity - run: | - git config --global user.name "github-actions[bot]" - git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com" - - - name: Sync Changelog - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - bazel run //tools/private/release -- sync-changelog \ - --remote origin \ - --no-dry-run - + uses: ./.github/workflows/release_sync_changelog.yaml + secrets: inherit - complete_sync_changelog: + call_complete_sync_changelog: if: | github.event.pull_request.merged == true && contains(github.event.pull_request.labels.*.name, 'type: sync-changelog') - runs-on: ubuntu-latest - permissions: - contents: write - issues: write - pull-requests: read - steps: - - name: Checkout repository - uses: actions/checkout@v7 - with: - fetch-depth: 0 - - - name: Setup Bazel - uses: bazel-contrib/setup-bazel@0.19.0 - with: - bazelisk-version: 1.20.0 - - - name: Complete Sync Changelog - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.pull_request.number }} - run: | - bazel run //tools/private/release -- complete-sync-changelog \ - --pr "$PR_NUMBER" + uses: ./.github/workflows/release_sync_changelog_complete.yaml + secrets: inherit diff --git a/.github/workflows/release_process_backports.yaml b/.github/workflows/release_process_backports.yaml index c613c5a3ca..8e221d643d 100644 --- a/.github/workflows/release_process_backports.yaml +++ b/.github/workflows/release_process_backports.yaml @@ -80,6 +80,6 @@ jobs: uses: ./.github/workflows/release_sync_changelog.yaml with: issue: ${{ inputs.issue }} - comment_id: ${{ inputs.comment_id }} secrets: inherit + diff --git a/.github/workflows/release_sync_changelog.yaml b/.github/workflows/release_sync_changelog.yaml index e095810486..3a7bdfe547 100644 --- a/.github/workflows/release_sync_changelog.yaml +++ b/.github/workflows/release_sync_changelog.yaml @@ -7,20 +7,12 @@ on: description: 'The Release Tracking Issue Number (e.g., 142)' required: false type: string - comment_id: - description: 'The ID of the comment that triggered this run (optional)' - required: false - type: string workflow_call: inputs: issue: description: 'The Release Tracking Issue Number (e.g., 142)' required: false type: string - comment_id: - description: 'The ID of the comment that triggered this run (optional)' - required: false - type: string permissions: contents: write @@ -49,19 +41,8 @@ jobs: - name: Sync Changelog to Main env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COMMENT_ID: ${{ inputs.comment_id }} ISSUE: ${{ inputs.issue }} run: | - ARGS=() - if [ -n "$ISSUE" ]; then - ISSUE="${ISSUE#\#}" - ARGS+=("--issue=$ISSUE") - fi - if [ -n "$COMMENT_ID" ]; then - ARGS+=("--triggering-comment=$COMMENT_ID") - fi - bazel run //tools/private/release -- sync-changelog \ - --remote origin \ - --no-dry-run \ - "${ARGS[@]}" + ${ISSUE:+--issue="$ISSUE"} \ + --remote origin diff --git a/.github/workflows/release_sync_changelog_complete.yaml b/.github/workflows/release_sync_changelog_complete.yaml new file mode 100644 index 0000000000..e210f3b5e1 --- /dev/null +++ b/.github/workflows/release_sync_changelog_complete.yaml @@ -0,0 +1,45 @@ +name: "Release: Sync Changelog: Complete" + +on: + workflow_dispatch: + inputs: + pr: + description: 'The merged sync-changelog PR number (optional; extracted from GITHUB_EVENT_PATH if omitted)' + required: false + type: string + workflow_call: + inputs: + pr: + description: 'The merged sync-changelog PR number (optional; extracted from GITHUB_EVENT_PATH if omitted)' + required: false + type: string + +permissions: + contents: write + issues: write + pull-requests: read + +jobs: + complete_sync_changelog: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Setup Bazel + uses: bazel-contrib/setup-bazel@0.19.0 + with: + bazelisk-version: 1.20.0 + + - name: Configure Git Identity + run: | + git config --global user.name "github-actions[bot]" + git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com" + + - name: Complete Sync Changelog + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + bazel run //tools/private/release -- complete-sync-changelog diff --git a/RELEASING.md b/RELEASING.md index c166d06fdf..6e3a72d009 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -174,7 +174,7 @@ UI: ```shell gh workflow run release_sync_changelog.yaml \ --repo bazel-contrib/rules_python \ - -f issue= + --raw-field issue= ``` Or comment `/sync-changelog` on the release tracking issue, or run via the @@ -182,7 +182,7 @@ release tool CLI: ```shell bazel run //tools/private/release -- \ - sync-changelog --issue --remote origin --no-dry-run + sync-changelog --issue --remote origin ``` ### Failure Behavior diff --git a/tests/tools/private/release/complete_sync_changelog_test.py b/tests/tools/private/release/complete_sync_changelog_test.py index d0eb148b57..51663e2b9c 100644 --- a/tests/tools/private/release/complete_sync_changelog_test.py +++ b/tests/tools/private/release/complete_sync_changelog_test.py @@ -1,4 +1,5 @@ import argparse +import json from tools.private.release.complete_sync_changelog import CompleteSyncChangelog @@ -49,6 +50,49 @@ def test_complete_sync_changelog_success(mock_gh): assert "- [ ] Sync Changelog #126 | status=pending pr=#888" in updated_body +def test_complete_sync_changelog_from_github_event_path(mock_gh, tmp_path, monkeypatch): + event_file = tmp_path / "event.json" + event_file.write_text( + json.dumps({"pull_request": {"number": 999}}), encoding="utf-8" + ) + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + + args = argparse.Namespace(pr=None) + mock_gh.prs[999] = { + "state": "MERGED", + "body": "Updates CHANGELOG.md\n\nRelease-Tracking-Issue: #123", + "mergeCommit": {"oid": "abcdef1234567890"}, + } + issue_body = """ +## Checklist +- [ ] Sync Changelog #124 | status=pending pr=#999 +""" + mock_gh.issues[123] = { + "title": "Release 2.1.0", + "body": issue_body, + "labels": ["type: release"], + "number": 123, + "url": "https://github.com/bazel-contrib/rules_python/issues/123", + } + + result = CompleteSyncChangelog(args, mock_gh).run() + + assert result == 0 + assert ( + "- [x] Sync Changelog #124 | status=done pr=#999 commit= abcdef12" + in mock_gh.get_issue_body(123) + ) + + +def test_complete_sync_changelog_missing_pr(mock_gh, monkeypatch): + monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) + args = argparse.Namespace(pr=None) + + result = CompleteSyncChangelog(args, mock_gh).run() + + assert result == 1 + + def test_complete_sync_changelog_not_merged(mock_gh): args = argparse.Namespace(pr=999) mock_gh.prs[999] = { diff --git a/tests/tools/private/release/sync_changelog_test.py b/tests/tools/private/release/sync_changelog_test.py index 4de642e9ff..a3b78afc61 100644 --- a/tests/tools/private/release/sync_changelog_test.py +++ b/tests/tools/private/release/sync_changelog_test.py @@ -1,4 +1,5 @@ import argparse +import json from unittest.mock import MagicMock, call from tools.private.release.gh import CreatePrError @@ -11,9 +12,7 @@ def test_sync_changelog_no_pending(mock_git, mock_gh): args = argparse.Namespace( issue=123, remote="origin", - dry_run=False, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -42,9 +41,7 @@ def test_sync_changelog_success(mocker, mock_git, mock_gh): args = argparse.Namespace( issue=123, remote="origin", - dry_run=False, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -72,7 +69,7 @@ def test_sync_changelog_success(mocker, mock_git, mock_gh): mock_git.checkout.assert_has_calls( [ call("main", track_remote="origin"), - call("prepare-2.0.0-backports-6affdae", create_branch=True), + call("sync-changelog-2.0.0-6affdae", create_branch=True), call("main"), ] ) @@ -86,7 +83,7 @@ def test_sync_changelog_success(mocker, mock_git, mock_gh): ) mock_git.push.assert_called_once_with( "origin", - "prepare-2.0.0-backports-6affdae", + "sync-changelog-2.0.0-6affdae", set_upstream=True, force=True, ) @@ -94,8 +91,16 @@ def test_sync_changelog_success(mocker, mock_git, mock_gh): updated_body = mock_gh.get_issue_body(123) assert "- [ ] Sync Changelog #124 | status=pending pr=#1001" in updated_body + assert len(mock_gh.issue_comments[123]) == 1 + assert ( + mock_gh.issue_comments[123][0] + == "Sync changelog PR created: https://github.com/bazel-contrib/rules_python/pull/1001" + ) -def test_sync_changelog_branch_exists(mocker, mock_git, mock_gh): + +def test_sync_changelog_from_github_event_path( + mocker, mock_git, mock_gh, tmp_path, monkeypatch +): mock_process_news_class = mocker.patch( "tools.private.release.sync_changelog.ProcessNews" ) @@ -103,12 +108,14 @@ def test_sync_changelog_branch_exists(mocker, mock_git, mock_gh): mock_process_news_instance.run.return_value = 0 mock_process_news_class.return_value = mock_process_news_instance + event_file = tmp_path / "event.json" + event_file.write_text(json.dumps({"inputs": {"issue": "123"}}), encoding="utf-8") + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + args = argparse.Namespace( - issue=123, + issue=None, remote="origin", - dry_run=False, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -118,24 +125,18 @@ def test_sync_changelog_branch_exists(mocker, mock_git, mock_gh): """, "labels": ["type: release"], } - mock_git.branch_exists.return_value = True - mock_git.get_commit_sha.return_value = "main_sha" mock_git.status.side_effect = ["", "M CHANGELOG.md"] result = SyncChangelog(args, mock_git, mock_gh).run() assert result == 0 - mock_git.checkout.assert_has_calls( - [ - call("main", track_remote="origin"), - call("prepare-2.0.0-backports-6affdae"), - call("main"), - ] + assert ( + "- [ ] Sync Changelog #124 | status=pending pr=#1001" + in mock_gh.get_issue_body(123) ) - mock_git.reset_hard.assert_called_once_with(reset_to="main") -def test_sync_changelog_dry_run(mocker, mock_git, mock_gh): +def test_sync_changelog_branch_exists(mocker, mock_git, mock_gh): mock_process_news_class = mocker.patch( "tools.private.release.sync_changelog.ProcessNews" ) @@ -146,9 +147,7 @@ def test_sync_changelog_dry_run(mocker, mock_git, mock_gh): args = argparse.Namespace( issue=123, remote="origin", - dry_run=True, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -158,15 +157,21 @@ def test_sync_changelog_dry_run(mocker, mock_git, mock_gh): """, "labels": ["type: release"], } - mock_git.get_commit_sha.return_value = "main_start_sha" - mock_git.status.side_effect = ["", "M CHANGELOG.md", "M CHANGELOG.md"] + mock_git.branch_exists.return_value = True + mock_git.get_commit_sha.return_value = "main_sha" + mock_git.status.side_effect = ["", "M CHANGELOG.md"] result = SyncChangelog(args, mock_git, mock_gh).run() assert result == 0 - mock_git.push.assert_not_called() - mock_git.commit.assert_not_called() - mock_git.reset_hard.assert_called_once_with(reset_to="main_start_sha") + mock_git.checkout.assert_has_calls( + [ + call("main", track_remote="origin"), + call("sync-changelog-2.0.0-6affdae"), + call("main"), + ] + ) + mock_git.reset_hard.assert_called_once_with(reset_to="main") def test_sync_changelog_auto_discover_issue(mocker, mock_git, mock_gh): @@ -180,9 +185,7 @@ def test_sync_changelog_auto_discover_issue(mocker, mock_git, mock_gh): args = argparse.Namespace( issue=None, remote="origin", - dry_run=False, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "number": 123, @@ -202,15 +205,14 @@ def test_sync_changelog_auto_discover_issue(mocker, mock_git, mock_gh): "- [ ] Sync Changelog #124 | status=pending pr=#1001" in mock_gh.get_issue_body(123) ) + assert len(mock_gh.issue_comments[123]) == 1 def test_sync_changelog_multiple_open_issues_fails(mock_git, mock_gh): args = argparse.Namespace( issue=None, remote="origin", - dry_run=False, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "number": 123, @@ -242,9 +244,7 @@ def test_sync_changelog_specific_prs_arg(mocker, mock_git, mock_gh): args = argparse.Namespace( issue=123, remote="origin", - dry_run=False, prs=["#124", "125"], - triggering_comment=None, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -275,9 +275,7 @@ def test_sync_changelog_no_changes(mocker, mock_git, mock_gh): args = argparse.Namespace( issue=123, remote="origin", - dry_run=False, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -308,9 +306,7 @@ def test_sync_changelog_process_news_failure(mocker, mock_git, mock_gh): args = argparse.Namespace( issue=123, remote="origin", - dry_run=False, prs=None, - triggering_comment=55555, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -325,7 +321,12 @@ def test_sync_changelog_process_news_failure(mocker, mock_git, mock_gh): result = SyncChangelog(args, mock_git, mock_gh).run() assert result == 1 - assert mock_gh.reactions.get(55555) == ["-1"] + assert len(mock_gh.issue_comments[123]) == 1 + assert ( + "Warning: Failed to create sync PR to main for backports" + in mock_gh.issue_comments[123][0] + ) + assert "Traceback" not in mock_gh.issue_comments[123][0] def test_sync_changelog_create_pr_failure(mocker, mock_git, mock_gh): @@ -339,9 +340,7 @@ def test_sync_changelog_create_pr_failure(mocker, mock_git, mock_gh): args = argparse.Namespace( issue=123, remote="origin", - dry_run=False, prs=None, - triggering_comment=None, ) mock_gh.issues[123] = { "title": "Release 2.0.0", @@ -359,3 +358,9 @@ def test_sync_changelog_create_pr_failure(mocker, mock_git, mock_gh): result = SyncChangelog(args, mock_git, mock_gh).run() assert result == 1 + assert len(mock_gh.issue_comments[123]) == 1 + assert ( + "Warning: Failed to create sync PR to main for backports" + in mock_gh.issue_comments[123][0] + ) + assert "Traceback" not in mock_gh.issue_comments[123][0] diff --git a/tools/private/release/complete_sync_changelog.py b/tools/private/release/complete_sync_changelog.py index 9c8d9028f0..9180506a09 100644 --- a/tools/private/release/complete_sync_changelog.py +++ b/tools/private/release/complete_sync_changelog.py @@ -1,13 +1,16 @@ """Subcommand to mark sync changelog tasks as complete.""" +import logging import re -from tools.private.release.gh import GitHub +from tools.private.release.gh import GitHub, get_github_event_pr_number from tools.private.release.release_issue import ( parse_checklist_state, update_task_in_body, ) +logger = logging.getLogger(__name__) + class CompleteSyncChangelog: """Class to mark sync changelog tasks as complete.""" @@ -19,31 +22,41 @@ def __init__(self, args, gh: GitHub): def run(self) -> int: """Executes the complete-sync-changelog subcommand.""" args = self.args - print(f"Completing sync changelog for PR #{args.pr}...") + pr_num = args.pr or get_github_event_pr_number() + if not pr_num: + logger.error( + "No PR specified and could not extract PR number from GITHUB_EVENT_PATH." + ) + return 1 + + logger.info("Completing sync changelog for PR #%d...", pr_num) - pr_info = self.gh.get_pr_info(args.pr) + pr_info = self.gh.get_pr_info(pr_num) if not pr_info or pr_info.get("state") != "MERGED": state = pr_info.get("state", "UNKNOWN") - print(f"Error: PR #{args.pr} is not merged yet (state: {state}).") + logger.error("PR #%d is not merged yet (state: %s).", pr_num, state) return 1 # Resolve issue number from PR body using Release-Tracking-Issue: # pr_body = pr_info.get("body") or "" match = re.search(r"Release-Tracking-Issue:\s*#(\d+)", pr_body) if not match: - print( - f"Error: Could not find 'Release-Tracking-Issue: #' in" - f" PR #{args.pr} body: {pr_body}" + logger.error( + "Could not find 'Release-Tracking-Issue: #' in PR #%d body: %s", + pr_num, + pr_body, ) return 1 issue_num = int(match.group(1)) - print(f"Resolved tracking issue #{issue_num} from PR #{args.pr} body.") + logger.info("Resolved tracking issue #%d from PR #%d body.", issue_num, pr_num) commit_sha = pr_info["mergeCommit"]["oid"] short_commit = commit_sha[:8] - print( - f"PR #{args.pr} merged at commit {commit_sha}. Updating tracking issue..." + logger.info( + "PR #%d merged at commit %s. Updating tracking issue...", + pr_num, + commit_sha, ) # Update checklist: mark all Sync Changelog tasks pointing to this PR as done @@ -52,14 +65,14 @@ def run(self) -> int: sync_changelogs = state.get("sync_changelogs", {}) updated_any = False - for pr_num, task in sync_changelogs.items(): + for target_pr_num, task in sync_changelogs.items(): # Check if this task points to our merged PR task_pr = task.metadata.get("pr") - if task_pr == f"#{args.pr}": - print(f"Marking task '{task.name}' as complete...") + if task_pr == f"#{pr_num}": + logger.info("Marking task '%s' as complete...", task.name) metadata = { "status": "done", - "pr": f"#{args.pr}", + "pr": f"#{pr_num}", "commit": short_commit, } body = update_task_in_body( @@ -68,11 +81,11 @@ def run(self) -> int: updated_any = True if not updated_any: - print(f"Warning: No 'Sync Changelog' tasks found pointing to PR #{args.pr}") + logger.warning("No 'Sync Changelog' tasks found pointing to PR #%d", pr_num) return 0 self.gh.update_issue_body(issue_num, body) - print("Sync changelog tasks marked complete successfully!") + logger.info("Sync changelog tasks marked complete successfully!") return 0 @classmethod @@ -85,8 +98,7 @@ def add_parser(cls, subparsers): parser.add_argument( "--pr", type=int, - required=True, - help="The merged sync changelog PR number.", + help="The merged sync changelog PR number (optional; extracted from GITHUB_EVENT_PATH if omitted).", ) parser.set_defaults(command=cls.run_from_args) diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py index bde969068b..064368dfa6 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -19,6 +19,7 @@ RELEASE_LABEL = "type: release" BACKPORT_LABEL = "type: backport-pr" RELEASE_PREPARED_LABEL = "release-prepared" +SYNC_CHANGELOG_LABEL = "type: sync-changelog" # GitHub reaction types # See: https://docs.github.com/en/rest/reactions/reactions?apiVersion=2022-11-28#about-reactions @@ -107,10 +108,98 @@ class PrDict(TypedDict, total=False): state: str isDraft: bool mergeCommit: dict[str, str] - auto_merge: AutoMergeDict | None + auto_merge: AutoMergeDict files: list[PrFileDict] +class GitHubEventPullRequestDict(TypedDict, total=False): + """Pull request object in a GitHub Actions event payload. + + See GitHub Webhook events docs: + https://docs.github.com/en/webhooks/webhook-events-and-payloads#pull_request + """ + + number: int + + +class GitHubEventIssueDict(TypedDict, total=False): + """Issue object in a GitHub Actions event payload. + + See GitHub Webhook events docs: + https://docs.github.com/en/webhooks/webhook-events-and-payloads#issues + """ + + number: int + + +class GitHubEventDict(TypedDict, total=False): + """Representation of a GitHub Actions event payload JSON ($GITHUB_EVENT_PATH). + + See GitHub Actions events docs: + https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows + """ + + inputs: dict[str, object] + pull_request: GitHubEventPullRequestDict + issue: GitHubEventIssueDict + number: int | None + + +def get_github_event_data() -> GitHubEventDict: + """Loads JSON data from GITHUB_EVENT_PATH if set.""" + event_path = os.environ.get("GITHUB_EVENT_PATH") + if not event_path or not os.path.isfile(event_path): + return {} + with open(event_path, "r", encoding="utf-8") as f: + return json.load(f) + + +def get_github_event_pr_number() -> int | None: + """Extracts PR number from GITHUB_EVENT_PATH if available.""" + data = get_github_event_data() + if not data: + return None + if ( + "inputs" in data + and isinstance(data["inputs"], dict) + and data["inputs"].get("pr") + ): + pr_val = str(data["inputs"]["pr"]).lstrip("#") + if pr_val.isdigit(): + return int(pr_val) + if ( + "pull_request" in data + and isinstance(data["pull_request"], dict) + and data["pull_request"].get("number") + ): + return int(data["pull_request"]["number"]) + if "number" in data and isinstance(data["number"], int): + return data["number"] + return None + + +def get_github_event_issue_number() -> int | None: + """Extracts Issue number from GITHUB_EVENT_PATH if available.""" + data = get_github_event_data() + if not data: + return None + if ( + "inputs" in data + and isinstance(data["inputs"], dict) + and data["inputs"].get("issue") + ): + issue_val = str(data["inputs"]["issue"]).lstrip("#") + if issue_val.isdigit(): + return int(issue_val) + if ( + "issue" in data + and isinstance(data["issue"], dict) + and data["issue"].get("number") + ): + return int(data["issue"]["number"]) + return None + + class MultipleTrackingIssuesError(ValueError): """Raised when multiple open tracking issues are found for a version.""" diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 51c18a3d1d..64fd118502 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -57,12 +57,32 @@ def create_parser(): return parser +class GitHubActionsLogHandler(logging.Handler): + """Outputs GitHub Actions workflow command annotations for log records.""" + + def emit(self, record: logging.LogRecord) -> None: + if record.levelno >= logging.ERROR: + prefix = "::error::" + elif record.levelno >= logging.WARNING: + prefix = "::warning::" + elif record.levelno >= logging.INFO: + prefix = "::notice::" + else: + return + try: + msg = record.getMessage() + print(f"{prefix}{msg}", file=sys.stdout, flush=True) + except Exception: + self.handleError(record) + + def main(): logging.basicConfig( format="%(levelname)s:%(filename)s:%(lineno)d: %(message)s", level=logging.INFO, stream=sys.stderr, ) + logging.getLogger().addHandler(GitHubActionsLogHandler()) print(f"sys.argv: {sys.argv}") if "BUILD_WORKSPACE_DIRECTORY" in os.environ: os.chdir(os.environ["BUILD_WORKSPACE_DIRECTORY"]) diff --git a/tools/private/release/sync_changelog.py b/tools/private/release/sync_changelog.py index 19a5339dae..85140b8c2b 100644 --- a/tools/private/release/sync_changelog.py +++ b/tools/private/release/sync_changelog.py @@ -3,12 +3,14 @@ import argparse import hashlib import logging +import os import traceback from tools.private.release.gh import ( - GH_REACTION_THUMBS_DOWN, + SYNC_CHANGELOG_LABEL, GitHub, GitHubInterface, + get_github_event_issue_number, ) from tools.private.release.git import Git from tools.private.release.process_news import ProcessNews @@ -17,10 +19,30 @@ parse_checklist_state, update_task_in_body, ) -from tools.private.release.utils import format_exception, parse_pr_list +from tools.private.release.utils import ( + format_exception, + parse_pr_list, +) logger = logging.getLogger(__name__) +SYNC_CHANGELOG_SUCCESS_COMMENT_TEMPLATE = "Sync changelog PR created: {pr_url}" + +SYNC_CHANGELOG_FAILURE_COMMENT_TEMPLATE = ( + "Warning: Failed to create sync PR to main for backports. {action_url_text}" +) + + +def _get_workflow_action_url_text(repo: str) -> str: + """Returns a link to the GitHub Actions run if available.""" + run_id = os.environ.get("GITHUB_RUN_ID") + server_url = os.environ.get("GITHUB_SERVER_URL", "https://github.com") + repository = os.environ.get("GITHUB_REPOSITORY", repo) + if run_id: + action_url = f"{server_url}/{repository}/actions/runs/{run_id}" + return f"See [workflow run]({action_url}) for logs." + return "See workflow logs." + class SyncChangelog: """Class to sync changelog to main for backports in a release.""" @@ -32,33 +54,32 @@ def __init__(self, args, git: Git, gh: GitHubInterface): def run(self) -> int: """Executes the sync-changelog subcommand.""" - args = self.args - exit_code = 0 try: - exit_code = self._run_internal() + return self._run_internal() except Exception as e: - logger.error("Unexpected error: %s", e) + logger.error("Unexpected error in sync-changelog: %s", format_exception(e)) traceback.print_exc() - exit_code = 1 + return 1 - if exit_code != 0 and args.triggering_comment: - logger.info( - "Reacting with thumbs-down to comment %s...", - args.triggering_comment, + def _post_failure_comment(self, issue_num: int) -> None: + """Posts a failure warning message to the tracking issue.""" + try: + action_url_text = _get_workflow_action_url_text(self.gh.repo) + comment_body = SYNC_CHANGELOG_FAILURE_COMMENT_TEMPLATE.format( + action_url_text=action_url_text, + ) + self.gh.post_issue_comment(issue_num, comment_body) + except Exception as e: + logger.warning( + "Failed to post warning comment to issue #%d: %s", + issue_num, + format_exception(e), ) - try: - self.gh.add_comment_reaction( - args.triggering_comment, GH_REACTION_THUMBS_DOWN - ) - except Exception as e: - logger.error("Failed to add reaction to comment: %s", e) - - return exit_code def _run_internal(self) -> int: """Internal implementation of sync-changelog.""" args = self.args - issue_num = args.issue + issue_num = args.issue or get_github_event_issue_number() if not issue_num: logger.info( "No issue specified. Auto-discovering open release tracking issue..." @@ -77,11 +98,25 @@ def _run_internal(self) -> int: logger.error("No open release tracking issues found.") return 1 + try: + return self._sync_for_issue(issue_num) + except Exception as e: + err_msg = format_exception(e) + logger.error( + "Failed to sync changelog for issue #%d: %s", issue_num, err_msg + ) + self._post_failure_comment(issue_num) + raise + + def _sync_for_issue(self, issue_num: int) -> int: + args = self.args body = self.gh.get_issue_body(issue_num) issue_title = self.gh.get_issue_title(issue_num) version_match = RELEASE_TITLE_RE.search(issue_title) if not version_match: - logger.error("Could not parse version from issue title: %s", issue_title) + err = f"Could not parse version from issue title: {issue_title}" + logger.error(err) + self._post_failure_comment(issue_num) return 1 version = version_match.group(1) @@ -93,11 +128,9 @@ def _run_internal(self) -> int: pr_num = self.gh.resolve_pr_number(pr_ref) pending_prs.append(pr_num) except Exception as e: - logger.error( - "Failed to resolve PR reference '%s': %s", - pr_ref, - format_exception(e), - ) + err = f"Failed to resolve PR reference '{pr_ref}': {format_exception(e)}" + logger.error(err) + self._post_failure_comment(issue_num) return 1 else: state = parse_checklist_state(body) @@ -121,10 +154,9 @@ def _run_internal(self) -> int: ) if self.git.status(): - logger.error( - "Git workspace is dirty. Please commit or stash changes" - " before running sync-changelog." - ) + err = "Git workspace is dirty. Please commit or stash changes before running sync-changelog." + logger.error(err) + self._post_failure_comment(issue_num) return 1 sorted_prs = sorted(pending_prs) @@ -132,25 +164,17 @@ def _run_internal(self) -> int: prs_hash = hashlib.sha256(prs_str.encode()).hexdigest()[:7] main_branch = "main" - backport_branch = f"prepare-{version}-backports-{prs_hash}" + sync_branch = f"sync-changelog-{version}-{prs_hash}" self.git.fetch(args.remote, refspec=main_branch) self.git.checkout(main_branch, track_remote=args.remote) - main_start_sha = self.git.get_commit_sha("HEAD") try: - if args.dry_run: - logger.info( - "[DRY RUN] Would create and checkout branch %s from %s", - backport_branch, - main_branch, - ) + if self.git.branch_exists(sync_branch): + self.git.checkout(sync_branch) + self.git.reset_hard(reset_to=main_branch) else: - if self.git.branch_exists(backport_branch): - self.git.checkout(backport_branch) - self.git.reset_hard(reset_to=main_branch) - else: - self.git.checkout(backport_branch, create_branch=True) + self.git.checkout(sync_branch, create_branch=True) # Run ProcessNews to process news files and version markers process_news_args = argparse.Namespace( @@ -160,98 +184,86 @@ def _run_internal(self) -> int: process_news_runner = ProcessNews(process_news_args, gh=self.gh) ret = process_news_runner.run() if ret != 0: - logger.error("ProcessNews failed for targets: %s", sorted_prs) + err = f"ProcessNews failed for targets: {sorted_prs}" + logger.error(err) + self._post_failure_comment(issue_num) return 1 if not self.git.status(): logger.info("No changes to sync after running process-news.") return 0 - if args.dry_run: - logger.info( - "[DRY RUN] Would commit: 'chore(release): sync changelog" - " for v%s backports'", - version, - ) - logger.info( - "[DRY RUN] Would push %s to %s", - backport_branch, - args.remote, - ) - logger.info( - "[DRY RUN] Would create PR to %s with label 'type: sync-changelog'", - main_branch, + self.git.add_modified_and_deleted() + self.git.commit(f"chore(release): sync changelog for v{version} backports") + self.git.push(args.remote, sync_branch, set_upstream=True, force=True) + + pr_title = f"chore(release): sync changelog for v{version} backports" + pr_body_lines = [ + "Updates CHANGELOG.md and removes news files for backports:", + ] + for pr_num in sorted_prs: + pr_body_lines.append(f"- #{pr_num}") + + pr_body_lines.append("") + pr_body_lines.append(f"Work towards #{issue_num}") + pr_body_lines.append(f"Release-Tracking-Issue: #{issue_num}") + pr_body = "\n".join(pr_body_lines) + + logger.info("Creating PR to %s...", main_branch) + pr_url = self.gh.create_pr( + title=pr_title, + body=pr_body, + base=main_branch, + labels=[SYNC_CHANGELOG_LABEL], + ) + logger.info("Created PR: %s", pr_url) + + pr_num = int(pr_url.split("/")[-1]) + try: + logger.info("Enabling auto-merge for PR #%s...", pr_num) + self.gh.enable_auto_merge(pr_num) + except Exception as e: + logger.warning( + "Failed to enable auto-merge on PR #%s: %s", + pr_num, + format_exception(e), ) + + try: logger.info( - "[DRY RUN] Would update tracking issue #%s checklist tasks" - " 'Sync Changelog #' to PENDING", + "Updating tracking issue #%s checklist with" + " Sync Changelog tasks...", issue_num, ) - logger.info("[DRY RUN] Diff of changes:\n%s", self.git.status()) - else: - self.git.add_modified_and_deleted() - self.git.commit( - f"chore(release): sync changelog for v{version} backports" - ) - self.git.push( - args.remote, backport_branch, set_upstream=True, force=True + issue_body = self.gh.get_issue_body(issue_num) + for pr in sorted_prs: + task_name = f"Sync Changelog #{pr}" + metadata = {"status": "pending", "pr": f"#{pr_num}"} + issue_body = update_task_in_body( + issue_body, + task_name, + checked=False, + metadata=metadata, + ) + self.gh.update_issue_body(issue_num, issue_body) + except Exception as e: + logger.warning( + "Failed to update tracking issue checklist: %s", + format_exception(e), ) - pr_title = f"chore(release): sync changelog for v{version} backports" - pr_body_lines = [ - "Updates CHANGELOG.md and removes news files for backports:", - ] - for pr_num in sorted_prs: - pr_body_lines.append(f"- #{pr_num}") - - pr_body_lines.append("") - pr_body_lines.append(f"Work towards #{issue_num}") - pr_body_lines.append(f"Release-Tracking-Issue: #{issue_num}") - pr_body = "\n".join(pr_body_lines) - - logger.info("Creating PR to %s...", main_branch) - pr_url = self.gh.create_pr( - title=pr_title, - body=pr_body, - base=main_branch, - labels=["type: sync-changelog"], + try: + success_body = SYNC_CHANGELOG_SUCCESS_COMMENT_TEMPLATE.format( + pr_url=pr_url, ) - logger.info("Created PR: %s", pr_url) - - try: - pr_num = int(pr_url.split("/")[-1]) - logger.info("Enabling auto-merge for PR #%s...", pr_num) - self.gh.enable_auto_merge(pr_num) - - logger.info( - "Updating tracking issue #%s checklist with" - " Sync Changelog tasks...", - issue_num, - ) - issue_body = self.gh.get_issue_body(issue_num) - for pr in sorted_prs: - task_name = f"Sync Changelog #{pr}" - metadata = {"status": "pending", "pr": f"#{pr_num}"} - issue_body = update_task_in_body( - issue_body, - task_name, - checked=False, - metadata=metadata, - ) - self.gh.update_issue_body(issue_num, issue_body) - except Exception as e: - logger.warning( - "Failed to update tracking issue or enable auto-merge: %s", - format_exception(e), - ) - finally: - if args.dry_run: - logger.info( - "[DRY RUN] Resetting branch %s to %s after changelog sync dry run", - main_branch, - main_start_sha, + self.gh.post_issue_comment(issue_num, success_body) + except Exception as e: + logger.warning( + "Failed to post success comment to issue #%s: %s", + issue_num, + format_exception(e), ) - self.git.reset_hard(reset_to=main_start_sha) + finally: self.git.checkout(main_branch) return 0 @@ -266,7 +278,7 @@ def add_parser(cls, subparsers): parser.add_argument( "--issue", type=int, - help="The tracking issue number (optional; auto-discovered if omitted).", + help="The tracking issue number (optional; extracted from GITHUB_EVENT_PATH or auto-discovered if omitted).", ) parser.add_argument( "--remote", @@ -282,17 +294,6 @@ def add_parser(cls, subparsers): " separated) to sync (optional)." ), ) - parser.add_argument( - "--triggering-comment", - type=int, - help="The ID of the comment that triggered this run (optional).", - ) - parser.add_argument( - "--dry-run", - action=argparse.BooleanOptionalAction, - default=True, - help="Perform a dry run (default: True). Use --no-dry-run to actually execute.", - ) parser.set_defaults(command=cls.run_from_args) @classmethod From fd05c478654861cc0850dc7ee6df005b3feedb15 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 17 Aug 2026 07:32:24 +0000 Subject: [PATCH 3/5] refactor(release): dispatch PR closed workflows via on_pr_closed parser and add gha test fixture Refactor PR closed event handling to use a dedicated Python parsing script and introduce a reusable GitHub Actions test fixture. Why: - Align on_pr_closed workflow structure with on_comment.yaml using a separate parser step that reads GITHUB_EVENT_PATH. - Remove redundant sync-changelog trigger on PR close since sync PRs are created during backport handling. - Simplify GHA environment setup in release tool unit tests with a shared fixture. How: - Add .github/workflows/on_pr_closed.py and update on_pr_closed.yaml to dispatch to process_backports or release_sync_changelog_complete.yaml. - Remove CLI --issue argument from release_sync_changelog.yaml and rename step to "Create Sync PR to Main". - Introduce gha test fixture in release_test_helper.py and add on_pr_closed_test.py. --- .github/workflows/on_pr_closed.py | 124 ++++++++++++++++ .github/workflows/on_pr_closed.yaml | 56 +++----- .github/workflows/release_sync_changelog.yaml | 4 +- .../release/complete_sync_changelog_test.py | 13 +- .../private/release/release_test_helper.py | 58 ++++++++ .../private/release/sync_changelog_test.py | 9 +- tests/workflows/BUILD.bazel | 18 +++ tests/workflows/on_pr_closed_test.py | 133 ++++++++++++++++++ 8 files changed, 357 insertions(+), 58 deletions(-) create mode 100644 .github/workflows/on_pr_closed.py create mode 100644 tests/workflows/on_pr_closed_test.py diff --git a/.github/workflows/on_pr_closed.py b/.github/workflows/on_pr_closed.py new file mode 100644 index 0000000000..0d437abec8 --- /dev/null +++ b/.github/workflows/on_pr_closed.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Parses closed/merged PR events to dispatch release and backport workflows.""" + +import json +import os +import re +import subprocess +import sys + + +def _load_event_data() -> dict: + """Loads event JSON payload from GITHUB_EVENT_PATH.""" + event_path = os.environ.get("GITHUB_EVENT_PATH") + if not event_path or not os.path.isfile(event_path): + return {} + try: + with open(event_path, "r", encoding="utf-8") as f: + return json.load(f) + except Exception: + return {} + + +def _write_github_output(key: str, value: str) -> None: + """Appends key=value to $GITHUB_OUTPUT.""" + path = os.environ.get("GITHUB_OUTPUT") + if path: + with open(path, "a", encoding="utf-8") as f: + f.write(f"{key}={value}\n") + + +def _check_active_release_issue(repo: str) -> bool: + """Checks if there is any active release tracking issue open.""" + cmd = [ + "gh", + "issue", + "list", + "--label", + "type: release", + "--state", + "open", + "--json", + "number", + ] + if repo: + cmd.extend(["--repo", repo]) + res = subprocess.run(cmd, capture_output=True, text=True, check=False) + if res.returncode != 0: + return False + try: + issues = json.loads(res.stdout or "[]") + return bool(issues) + except Exception: + return False + + +def _check_pr_has_backport_comment(repo: str, pr_number: str) -> bool: + """Checks if PR comments contain a /backport command.""" + cmd = ["gh", "pr", "view", pr_number, "--json", "comments"] + if repo: + cmd.extend(["--repo", repo]) + res = subprocess.run(cmd, capture_output=True, text=True, check=False) + if res.returncode != 0: + return False + try: + data = json.loads(res.stdout or "{}") + comments = data.get("comments", []) + return any( + re.search(r"^\s*/backport(?:\s|$)", c.get("body", ""), re.MULTILINE) + for c in comments + ) + except Exception: + return False + + +def process_pr_closed() -> int: + """Processes closed PR event and determines workflow to dispatch.""" + event = _load_event_data() + pr_data = event.get("pull_request") + if not pr_data or not isinstance(pr_data, dict): + _write_github_output("command", "none") + return 0 + + is_merged = bool(pr_data.get("merged", False)) + pr_number = str(pr_data.get("number") or event.get("number") or "") + + if not is_merged or not pr_number: + _write_github_output("command", "none") + return 0 + + repo = event.get("repository", {}).get("full_name") or os.environ.get( + "GITHUB_REPOSITORY", "" + ) + + labels_data = pr_data.get("labels", []) + labels = [] + if isinstance(labels_data, list): + for label in labels_data: + if isinstance(label, dict) and "name" in label: + labels.append(label["name"]) + elif isinstance(label, str): + labels.append(label) + + if "type: sync-changelog" in labels: + _write_github_output("command", "complete-sync-changelog") + _write_github_output("pr_number", pr_number) + return 0 + + if _check_active_release_issue(repo) and _check_pr_has_backport_comment( + repo, pr_number + ): + _write_github_output("command", "process-backports") + _write_github_output("pr_number", pr_number) + return 0 + + _write_github_output("command", "none") + return 0 + + +def _main() -> None: + sys.exit(process_pr_closed()) + + +if __name__ == "__main__": + _main() diff --git a/.github/workflows/on_pr_closed.yaml b/.github/workflows/on_pr_closed.yaml index 1c2b273d04..c5f5b40447 100644 --- a/.github/workflows/on_pr_closed.yaml +++ b/.github/workflows/on_pr_closed.yaml @@ -5,8 +5,8 @@ on: types: [closed] permissions: - contents: read - issues: read + contents: write + issues: write pull-requests: read jobs: @@ -17,40 +17,26 @@ jobs: steps: - run: echo "No-op" - check_if_backport: + parse_pr: runs-on: ubuntu-latest if: github.event.pull_request.merged == true outputs: - should_process: ${{ steps.check.outputs.should_process }} + command: ${{ steps.parse.outputs.command }} + pr_number: ${{ steps.parse.outputs.pr_number }} steps: - - name: Check if PR is a backport candidate - id: check + - uses: actions/checkout@v7 + - uses: actions/setup-python@v5 + with: + python-version: "3.14" + - name: Parse PR + id: parse env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.pull_request.number }} - run: | - # Check if there is any active release issue - ACTIVE_ISSUES=$(gh issue list --repo ${{ github.repository }} --label "type: release" --state open --json number) - if [ "$ACTIVE_ISSUES" = "[]" ] || [ -z "$ACTIVE_ISSUES" ]; then - echo "No active release tracking issue found. Skipping." - echo "should_process=false" >> "$GITHUB_OUTPUT" - exit 0 - fi - - # Check if PR has "/backport" in comments (only comments, not body) - PR_DATA=$(gh pr view "$PR_NUMBER" --repo ${{ github.repository }} --json comments) - - if echo "$PR_DATA" | jq -r '.comments[].body' | grep -qE '^[[:space:]]*/backport([[:space:]]|$)'; then - echo "Found /backport comment. Proceeding." - echo "should_process=true" >> "$GITHUB_OUTPUT" - else - echo "No /backport comment found. Skipping." - echo "should_process=false" >> "$GITHUB_OUTPUT" - fi + run: .github/workflows/on_pr_closed.py process_backports: - needs: check_if_backport - if: needs.check_if_backport.outputs.should_process == 'true' + needs: parse_pr + if: needs.parse_pr.outputs.command == 'process-backports' runs-on: ubuntu-latest permissions: contents: write @@ -75,23 +61,15 @@ jobs: - name: Process Backports env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.pull_request.number }} + PR_NUMBER: ${{ needs.parse_pr.outputs.pr_number }} run: | bazel run //tools/private/release -- on-pr-merged \ "$PR_NUMBER" \ --remote origin \ --no-dry-run - call_sync_changelog: - needs: process_backports - uses: ./.github/workflows/release_sync_changelog.yaml - secrets: inherit - call_complete_sync_changelog: - if: | - github.event.pull_request.merged == true && - contains(github.event.pull_request.labels.*.name, 'type: sync-changelog') + needs: parse_pr + if: needs.parse_pr.outputs.command == 'complete-sync-changelog' uses: ./.github/workflows/release_sync_changelog_complete.yaml secrets: inherit - - diff --git a/.github/workflows/release_sync_changelog.yaml b/.github/workflows/release_sync_changelog.yaml index 3a7bdfe547..70f3cf8f04 100644 --- a/.github/workflows/release_sync_changelog.yaml +++ b/.github/workflows/release_sync_changelog.yaml @@ -38,11 +38,9 @@ jobs: git config --global user.name "github-actions[bot]" git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com" - - name: Sync Changelog to Main + - name: Create Sync PR to Main env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - ISSUE: ${{ inputs.issue }} run: | bazel run //tools/private/release -- sync-changelog \ - ${ISSUE:+--issue="$ISSUE"} \ --remote origin diff --git a/tests/tools/private/release/complete_sync_changelog_test.py b/tests/tools/private/release/complete_sync_changelog_test.py index 51663e2b9c..6948dc54ca 100644 --- a/tests/tools/private/release/complete_sync_changelog_test.py +++ b/tests/tools/private/release/complete_sync_changelog_test.py @@ -1,5 +1,4 @@ import argparse -import json from tools.private.release.complete_sync_changelog import CompleteSyncChangelog @@ -50,12 +49,8 @@ def test_complete_sync_changelog_success(mock_gh): assert "- [ ] Sync Changelog #126 | status=pending pr=#888" in updated_body -def test_complete_sync_changelog_from_github_event_path(mock_gh, tmp_path, monkeypatch): - event_file = tmp_path / "event.json" - event_file.write_text( - json.dumps({"pull_request": {"number": 999}}), encoding="utf-8" - ) - monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) +def test_complete_sync_changelog_from_github_event_path(mock_gh, gha): + gha.set_event(pr=999) args = argparse.Namespace(pr=None) mock_gh.prs[999] = { @@ -84,8 +79,8 @@ def test_complete_sync_changelog_from_github_event_path(mock_gh, tmp_path, monke ) -def test_complete_sync_changelog_missing_pr(mock_gh, monkeypatch): - monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) +def test_complete_sync_changelog_missing_pr(mock_gh, gha): + gha.clear_event() args = argparse.Namespace(pr=None) result = CompleteSyncChangelog(args, mock_gh).run() diff --git a/tests/tools/private/release/release_test_helper.py b/tests/tools/private/release/release_test_helper.py index 064b5c260c..b7abd5e727 100644 --- a/tests/tools/private/release/release_test_helper.py +++ b/tests/tools/private/release/release_test_helper.py @@ -1,4 +1,5 @@ import dataclasses +import json import shutil from pathlib import Path from unittest.mock import MagicMock, patch @@ -22,6 +23,57 @@ class ReleaseToolEnv: github_output_file: Path +class GitHubActionsHelper: + """Helper for mocking GitHub Actions environment and events.""" + + def __init__(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + self.tmp_path = tmp_path + self.monkeypatch = monkeypatch + self.event_file = tmp_path / "github_event.json" + self.output_file = tmp_path / "github_output" + self.monkeypatch.setenv("GITHUB_OUTPUT", str(self.output_file)) + + def set_event( + self, + *, + inputs: dict[str, object] | None = None, + issue: int | dict[str, object] | None = None, + pr: int | dict[str, object] | None = None, + comment: str | dict[str, object] | None = None, + raw_payload: dict[str, object] | None = None, + ) -> Path: + """Configures GITHUB_EVENT_PATH with the specified event payload.""" + payload: dict[str, object] = {} + if raw_payload: + payload.update(raw_payload) + + if inputs is not None: + payload["inputs"] = inputs + + if isinstance(issue, int): + payload["issue"] = {"number": issue} + elif isinstance(issue, dict): + payload["issue"] = issue + + if isinstance(pr, int): + payload["pull_request"] = {"number": pr} + elif isinstance(pr, dict): + payload["pull_request"] = pr + + if isinstance(comment, str): + payload["comment"] = {"body": comment} + elif isinstance(comment, dict): + payload["comment"] = comment + + self.event_file.write_text(json.dumps(payload), encoding="utf-8") + self.monkeypatch.setenv("GITHUB_EVENT_PATH", str(self.event_file)) + return self.event_file + + def clear_event(self) -> None: + """Clears GITHUB_EVENT_PATH environment variable.""" + self.monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False) + + def _find_real_template_path() -> Path: r = runfiles.CreateOrRaise() path = r.Rlocation( @@ -54,6 +106,12 @@ def fixture_mock_gh(): return MockGitHub() +@pytest.fixture(name="gha") +def fixture_gha(tmp_path, monkeypatch): + """Fixture providing GitHub Actions environment helper.""" + return GitHubActionsHelper(tmp_path=tmp_path, monkeypatch=monkeypatch) + + @pytest.fixture(name="release_tool_env") def fixture_release_tool_env(tmp_path, monkeypatch): """Fixture providing a temp cwd with release template set up.""" diff --git a/tests/tools/private/release/sync_changelog_test.py b/tests/tools/private/release/sync_changelog_test.py index a3b78afc61..9e7bc4af39 100644 --- a/tests/tools/private/release/sync_changelog_test.py +++ b/tests/tools/private/release/sync_changelog_test.py @@ -1,5 +1,4 @@ import argparse -import json from unittest.mock import MagicMock, call from tools.private.release.gh import CreatePrError @@ -98,9 +97,7 @@ def test_sync_changelog_success(mocker, mock_git, mock_gh): ) -def test_sync_changelog_from_github_event_path( - mocker, mock_git, mock_gh, tmp_path, monkeypatch -): +def test_sync_changelog_from_github_event_path(mocker, mock_git, mock_gh, gha): mock_process_news_class = mocker.patch( "tools.private.release.sync_changelog.ProcessNews" ) @@ -108,9 +105,7 @@ def test_sync_changelog_from_github_event_path( mock_process_news_instance.run.return_value = 0 mock_process_news_class.return_value = mock_process_news_instance - event_file = tmp_path / "event.json" - event_file.write_text(json.dumps({"inputs": {"issue": "123"}}), encoding="utf-8") - monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + gha.set_event(inputs={"issue": "123"}) args = argparse.Namespace( issue=None, diff --git a/tests/workflows/BUILD.bazel b/tests/workflows/BUILD.bazel index d5809a6d1c..13d3ebbd01 100644 --- a/tests/workflows/BUILD.bazel +++ b/tests/workflows/BUILD.bazel @@ -19,3 +19,21 @@ pytest_test( "@pypi//pytest_mock", ], ) + +py_library( + name = "on_pr_closed", + testonly = True, + srcs = ["//.github/workflows:on_pr_closed.py"], + imports = ["../../.github/workflows"], + target_compatible_with = NOT_WINDOWS, +) + +pytest_test( + name = "on_pr_closed_test", + srcs = ["on_pr_closed_test.py"], + target_compatible_with = NOT_WINDOWS, + deps = [ + ":on_pr_closed", + "@pypi//pytest_mock", + ], +) diff --git a/tests/workflows/on_pr_closed_test.py b/tests/workflows/on_pr_closed_test.py new file mode 100644 index 0000000000..5ae84f542b --- /dev/null +++ b/tests/workflows/on_pr_closed_test.py @@ -0,0 +1,133 @@ +"""Tests for .github/workflows/on_pr_closed.py.""" + +import dataclasses +import json +from pathlib import Path + +import pytest +from on_pr_closed import ( + _main, + process_pr_closed, +) + + +@dataclasses.dataclass +class GitHubActionEnv: + output_file: Path + event_file: Path + + def read_outputs(self) -> dict[str, str]: + if not self.output_file.exists(): + return {} + res = {} + for line in self.output_file.read_text().splitlines(): + if "=" in line: + k, v = line.split("=", 1) + res[k] = v + return res + + def set_event( + self, + *, + pr_number: int = 123, + merged: bool = True, + labels: list[str] | None = None, + repo: str = "bazel-contrib/rules_python", + ) -> None: + payload = { + "pull_request": { + "number": pr_number, + "merged": merged, + "labels": [{"name": label} for label in (labels or [])], + }, + "repository": { + "full_name": repo, + }, + } + self.event_file.write_text(json.dumps(payload), encoding="utf-8") + + +@pytest.fixture(name="gha_env", autouse=True) +def fixture_gha_env(tmp_path, monkeypatch) -> GitHubActionEnv: + """Fixture that sets GITHUB_OUTPUT and GITHUB_EVENT_PATH environment variables.""" + out_file = tmp_path / "github_output.txt" + event_file = tmp_path / "github_event.json" + monkeypatch.setenv("GITHUB_OUTPUT", str(out_file)) + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_file)) + return GitHubActionEnv(output_file=out_file, event_file=event_file) + + +def test_pr_not_merged_ignored(gha_env): + gha_env.set_event(pr_number=123, merged=False) + process_pr_closed() + assert gha_env.read_outputs() == {"command": "none"} + + +def test_sync_changelog_pr_merged(gha_env): + gha_env.set_event( + pr_number=123, + merged=True, + labels=["type: sync-changelog"], + ) + process_pr_closed() + assert gha_env.read_outputs() == { + "command": "complete-sync-changelog", + "pr_number": "123", + } + + +def test_backport_candidate_pr_merged(mocker, gha_env): + mocker.patch("on_pr_closed._check_active_release_issue", return_value=True) + mocker.patch("on_pr_closed._check_pr_has_backport_comment", return_value=True) + + gha_env.set_event( + pr_number=123, + merged=True, + labels=["type: bug"], + ) + process_pr_closed() + assert gha_env.read_outputs() == { + "command": "process-backports", + "pr_number": "123", + } + + +def test_regular_pr_merged_no_backport(mocker, gha_env): + mocker.patch("on_pr_closed._check_active_release_issue", return_value=True) + mocker.patch("on_pr_closed._check_pr_has_backport_comment", return_value=False) + + gha_env.set_event( + pr_number=123, + merged=True, + labels=["type: feature"], + ) + process_pr_closed() + assert gha_env.read_outputs() == {"command": "none"} + + +def test_backport_pr_no_active_release(mocker, gha_env): + mocker.patch("on_pr_closed._check_active_release_issue", return_value=False) + + gha_env.set_event( + pr_number=123, + merged=True, + labels=["type: bug"], + ) + process_pr_closed() + assert gha_env.read_outputs() == {"command": "none"} + + +def test_main_cli_execution(gha_env): + gha_env.set_event( + pr_number=42, + merged=True, + labels=["type: sync-changelog"], + ) + + with pytest.raises(SystemExit): + _main() + + assert gha_env.read_outputs() == { + "command": "complete-sync-changelog", + "pr_number": "42", + } From 64e4420d64d7de5173b281e1dbab5aad20c70292 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 17 Aug 2026 07:41:43 +0000 Subject: [PATCH 4/5] fix(release): annotate repo attribute on GitHubInterface for pyrefly Add repo attribute annotation on GitHubInterface to resolve pyrefly type checking in release_lib. Why: - Pyrefly strict type checking in CI caught missing repo attribute on GitHubInterface in sync_changelog.py. How: - Annotate repo: str on GitHubInterface in gh.py. - Provide default empty string fallback for repo argument in _get_workflow_action_url_text. --- tools/private/release/gh.py | 2 ++ tools/private/release/sync_changelog.py | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py index 064368dfa6..4e7abb9b4e 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -233,6 +233,8 @@ class InvalidPrRefError(ValueError): class GitHubInterface(abc.ABC): """Abstract interface for GitHub operations.""" + repo: str + @abc.abstractmethod def post_issue_comment(self, issue_num: int, comment_body: str) -> None: """Posts a comment on an issue or PR. diff --git a/tools/private/release/sync_changelog.py b/tools/private/release/sync_changelog.py index 85140b8c2b..7e775d3565 100644 --- a/tools/private/release/sync_changelog.py +++ b/tools/private/release/sync_changelog.py @@ -33,11 +33,13 @@ ) -def _get_workflow_action_url_text(repo: str) -> str: +def _get_workflow_action_url_text(repo: str = "") -> str: """Returns a link to the GitHub Actions run if available.""" run_id = os.environ.get("GITHUB_RUN_ID") server_url = os.environ.get("GITHUB_SERVER_URL", "https://github.com") - repository = os.environ.get("GITHUB_REPOSITORY", repo) + repository = ( + os.environ.get("GITHUB_REPOSITORY") or repo or "bazel-contrib/rules_python" + ) if run_id: action_url = f"{server_url}/{repository}/actions/runs/{run_id}" return f"See [workflow run]({action_url}) for logs." From 7f0a0d74c62492d14d91929cf4414e261404a2eb Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 17 Aug 2026 08:06:10 +0000 Subject: [PATCH 5/5] agents: update CI soft-fail handling and GHA event parsing rules Update agent skills and workflow rules for CI soft-failing jobs and GHA event parsing. Why: - Prevent merge shepherd from getting stuck on experimental/rolling soft-failing CI jobs. - Encourage workflow parsing scripts to read event payloads directly from GITHUB_EVENT_PATH. How: - Add soft-failing jobs guidance in merge-pr and monitor-ci-results skills. - Add Event Payload Resolution guideline in github_actions_workflows.md. --- .agents/rules/github_actions_workflows.md | 2 ++ .agents/skills/merge-pr/SKILL.md | 2 ++ .agents/skills/monitor-ci-results/SKILL.md | 1 + 3 files changed, 5 insertions(+) diff --git a/.agents/rules/github_actions_workflows.md b/.agents/rules/github_actions_workflows.md index 8e95dcee5b..00f2177ecc 100644 --- a/.agents/rules/github_actions_workflows.md +++ b/.agents/rules/github_actions_workflows.md @@ -12,6 +12,8 @@ globs: [".github/workflows/*.yml", ".github/workflows/*.yaml", ".github/*.yaml"] redundant shell parameter stripping or conversions. * Print console messages using GitHub workflow command syntax (e.g., `::error::`, `::warning::`, `::notice::`, `::group::`). +* **Event Payload Resolution**: Load event details directly from + `$GITHUB_EVENT_PATH` rather than passing fields via env vars or CLI flags. ## Workflow Python Scripts & Testing * **Test Location**: Place workflow tests under `tests/workflows/`. diff --git a/.agents/skills/merge-pr/SKILL.md b/.agents/skills/merge-pr/SKILL.md index ae3e2e3ad5..8be67821e4 100644 --- a/.agents/skills/merge-pr/SKILL.md +++ b/.agents/skills/merge-pr/SKILL.md @@ -19,6 +19,8 @@ When the user asks to merge a pull request (e.g., "merge PR ", "merge th (`retry_buildkite_jobs.py `) to automatically retry any transient network flakes (e.g., HTTP 504 gateway timeouts, downloader errors). + - **Soft-Failing Jobs**: Experimental Buildkite jobs (e.g. `*rolling*` + Bazel) are non-blocking soft failures; do not treat them as merge blockers. - When the PR is queued, actively discover the merge queue branch via `gh api repos/:owner/:repo/branches --jq '.[].name | select(test("gh-readonly-queue/.*/pr--"))'` and monitor commit statuses/Buildkite builds running on that temporary diff --git a/.agents/skills/monitor-ci-results/SKILL.md b/.agents/skills/monitor-ci-results/SKILL.md index e2475b55ad..92b1dca7a6 100644 --- a/.agents/skills/monitor-ci-results/SKILL.md +++ b/.agents/skills/monitor-ci-results/SKILL.md @@ -33,6 +33,7 @@ or when monitoring CI after PR updates: 3. **Failure Reporting**: When any GitHub check or Buildkite job completes with errors, `monitor_remote_ci.py` dispatches a high-priority notification message reporting the failed check back to your conversation. + - Soft failures (e.g. `*rolling*` Bazel) are non-blocking warnings; ignore. 4. **Subagent Analysis**: Upon receiving a failure notification message from the monitoring script, immediately launch a separate subagent using the `invoke_subagent` tool with the role "CI Failure Analyzer" to run the