Skip to content

feat(scripts): sync release branch into feature branches on schedule - #5248

Draft
wuhuizuo wants to merge 3 commits into
mainfrom
feat/feature-branch-sync
Draft

wuhuizuo wants to merge 3 commits into
mainfrom
feat/feature-branch-sync

Conversation

@wuhuizuo

@wuhuizuo wuhuizuo commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Keep the long-lived feature/release-8.5-fts branch of the whole v8.5 product line automatically up to date with its base release branch, while preserving each feature branch's own commits and its v8.5.*-fts git tags.

What changed and how does it work?

  • Add scripts/plugins/sync-branch.ts, which merges a source branch into one or more target branches through the GitHub Merge API:
    • 201 Created creates a regular merge commit on the target branch;
    • 204 No Content means the target already contains the source;
    • 409 Conflict opens (or reuses) a conflict resolution pull request and fails the job so it is visible.
  • Add configs/sync-branches.yaml, the repository mapping consumed by the job. The same script still accepts --owner/--repository/--source_branch/--target_branch for ad-hoc single-repository runs.
  • Add the Prow periodic job periodic-sync-feature-branches-with-release-8.5 (daily at 02:00 UTC) in prow-jobs/pingcap-qe/ci/periodics.yaml.
  • Add scripts/plugins/sync-branch.test.ts unit tests for the pure helpers.
  • Add docs/guides/feature-branch-sync.md and link it from docs/guides/README.md.

Synced repositories (feature/release-8.5-fts):

Repository Source branch
pingcap/tidb release-8.5
pingcap/tiflash release-8.5
pingcap/ticdc release-8.5
pingcap/kvproto release-8.5
pingcap/tipb release-8.5
PingCAP-QE/tidb-test release-8.5
tikv/tikv release-8.5
tikv/pd release-8.5
tikv/client-c release-8.5
tikv/client-go tidb-8.5

Because the sync uses a merge commit and never resets, rebases or force-pushes the target branch, the feature-specific commits stay reachable and existing git tags are not orphaned. The job never calls any tag API; v8.5.*-fts tags are still created by the normal feature-branch build/release flow (scripts/flow/build/versioning-strategy.ts).

Intended order of operations: v8.5.x is released on the base release branch -> the next scheduled sync merges it into feature/release-8.5-fts -> the feature branch release flow produces and tags v8.5.x-fts.

Check List

  • Unit tests: deno test --allow-net --allow-read scripts/plugins/sync-branch.test.ts
  • deno check, deno lint, deno fmt --check
  • yq parse of the modified Prow job YAML and the new config YAML
  • .ci/update-prow-job-kustomization.sh (no kustomization change needed)
  • Read-only dry run of the full config resolved all 10 entries

Test result

running 5 tests from ./scripts/plugins/sync-branch.test.ts
normalizeTargetBranches ... ok
normalizeSyncSpecs ... ok
buildMergeCommitMessage ... ok
buildConflictPullRequestTitle ... ok
buildConflictPullRequestBody ... ok

ok | 5 passed | 0 failed

Add `scripts/plugins/sync-branch.ts` and the Prow periodic job
`periodic-sync-branch-tidb-feature-release-8.5-fts` to keep
`pingcap/tidb` `feature/release-8.5-fts` up to date with `release-8.5`.

The sync uses the GitHub Merge API to create a regular merge commit, so
the feature branch commits and its `v8.5.*-fts` git tags are preserved.
On merge conflicts it opens a conflict resolution pull request and fails
the job.

Document the mechanism in `docs/guides/feature-branch-sync.md`.
@ti-chi-bot

ti-chi-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign wuhuizuo for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary

This PR introduces a script to automate syncing feature branches with their base release branch using the GitHub Merge API. A Prow periodic job schedules the syncing operation daily. The implementation avoids destructive operations like rebases or force-pushes, preserving both feature-specific commits and git tags. The code is well-structured overall, with detailed documentation, unit tests, and adherence to established practices. However, there are some opportunities for improvement in error handling, efficiency, and maintainability.


Critical Issues

  • Error handling for webhook notifications (lines 263–273 of sync-branch.ts):

    • If the webhook notification fails, the error is only logged, and the script continues execution. This could lead to undetected failures in alerting critical conflicts.
    • Suggested Solution:
      if (!response.ok) {
        throw new Error(`Failed to send notification: HTTP ${response.status}`);
      }
  • Missing retry mechanism for GitHub API calls (multiple locations in sync-branch.ts):

    • Network or transient errors may cause API calls (e.g., mergeSourceIntoTarget, branchExists) to fail prematurely. Without retries, the job might report a failure unnecessarily.
    • Suggested Solution: Wrap API calls in a retry mechanism. For example:
      async function retry<T>(fn: () => Promise<T>, retries = 3): Promise<T> {
        for (let attempt = 1; attempt <= retries; attempt++) {
          try {
            return await fn();
          } catch (error) {
            if (attempt === retries) throw error;
            await new Promise((resolve) => setTimeout(resolve, 1000));
          }
        }
      }

Code Improvements

  • Avoid duplicate conflict pull requests (lines 174–186 of sync-branch.ts):

    • The ensureConflictPullRequest function only checks for PRs where head matches the source branch and base matches the target branch. This could lead to duplicate PRs when multiple syncs are attempted with unresolved conflicts.
    • Suggested Solution:
      Improve filtering logic to include PRs where the title matches buildConflictPullRequestTitle(sourceBranch, targetBranch).
  • Optimize notification construction (lines 252–260 of sync-branch.ts):

    • The notification card is constructed inline. If reused across multiple places, centralizing this logic would improve readability and code reuse.
    • Suggested Solution:
      Factor out buildNotificationCard(title, message) into a helper function.
  • Reduce verbosity in dry-run mode (lines 126–128 of sync-branch.ts):

    • Logging each target branch in dry-run mode can clutter the console for large sets of branches.
    • Suggested Solution:
      Aggregate results into a summary log after processing all branches.

Best Practices

  • Unit test coverage for edge cases (sync-branch.test.ts):

    • The tests do not cover scenarios where:
      • API calls fail (e.g., 500 server errors).
      • The normalizeTargetBranches function is given unusual input (e.g., duplicate empty branches).
    • Suggested Solution: Add tests for these edge cases:
      Deno.test("normalizeTargetBranches handles empty inputs", () => {
        assertEquals(normalizeTargetBranches(["", ""]), []);
      });
  • Missing integration tests:

    • While unit tests exist, there are no integration tests to validate behavior with mock GitHub API responses.
    • Suggested Solution: Use a mocking library to simulate GitHub API responses for end-to-end tests.
  • Environment variable validation (lines 309–310 in sync-branch.ts):

    • Critical inputs like github_private_token are not validated for presence or format. Missing tokens could lead to runtime errors.
    • Suggested Solution:
      if (!github_private_token) {
        console.error("Missing GitHub API token.");
        Deno.exit(1);
      }

Conclusion

The PR is well-documented, functional, and adheres to best practices in many areas, but error handling and test coverage could be improved. Addressing the critical issues and suggested improvements would enhance resilience and maintainability.

@ti-chi-bot ti-chi-bot Bot added the size/XXL label Sep 16, 2026
@wuhuizuo
wuhuizuo marked this pull request as draft September 16, 2026 08:32
@wuhuizuo

Copy link
Copy Markdown
Contributor Author

Keeping this pull request as a draft on purpose.

Please do not enable or merge it yet. We will mark it ready for review and merge it after the initial v8.5.x-fts release is delivered to the customer for testing in mid-October 2026. Until then the scheduled branch sync should not start running.

When we are ready to enable it:

  • mark the pull request as ready for review and merge it
  • make sure the feature/release-8.5-fts branch exists and the bot token (github-token) is allowed to push to it (bypass branch protection if needed)
  • optionally wire conflict notifications by adding --notify_webhook_url=<lark webhook> to the periodic job

Drive the branch sync from `scripts/plugins/sync-branches.yaml` so the
scheduled job keeps `feature/release-8.5-fts` up to date with its base
release branch for the whole v8.5 product line:

- pingcap/tidb, pingcap/tiflash, pingcap/ticdc, pingcap/kvproto,
  pingcap/tipb, PingCAP-QE/tidb-test, tikv/tikv, tikv/pd, tikv/client-c
  from `release-8.5`
- tikv/client-go from `tidb-8.5`

Rename the periodic job to
`periodic-sync-feature-branches-with-release-8.5` and pass `--config`.
The single-repository CLI arguments are kept for ad-hoc runs.
Move `scripts/plugins/sync-branches.yaml` to `configs/sync-branches.yaml`
and update the Prow job and the guide to the new path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant