From d24ae7de98ae7590bc191c67a2357c059d33fc9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20T=C3=B6lle?= Date: Sat, 14 Jun 2025 15:23:05 +0200 Subject: [PATCH] feat: detect changed pull request description and retry process (#197) If the release PR description was changed by a human after releaser-pleaser fetched the PR for the first time, releaser-pleaser would revert the users changes accidentally. This commit introduces an additional check right before updating the pull request description, to make sure we do not accidentally loose user changes. There is still the potential for a conflict in between us checking the description is the same, and updating the description. The time window for this should be reduced from multiple seconds-minutes to a few hundred milliseconds at most. In case a conflict is detected, we retry the whole process up to 2 times, to make sure that the users changes are reflected as soon as possible. This is especially important on GitLab CI/CD because a changed pull (merge) request description does not cause another job to run. With this change, the branch is still pushed, as the user is not expected to make any changes to it. Fixes #151 --- releaserpleaser.go | 58 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/releaserpleaser.go b/releaserpleaser.go index 409d8bb..a09aefe 100644 --- a/releaserpleaser.go +++ b/releaserpleaser.go @@ -2,6 +2,7 @@ package rp import ( "context" + "errors" "fmt" "log/slog" @@ -18,6 +19,14 @@ const ( PullRequestBranchFormat = "releaser-pleaser--branches--%s" ) +const ( + PullRequestConflictAttempts = 3 +) + +var ( + ErrorPullRequestConflict = errors.New("conflict: pull request description was changed while releaser-pleaser was running") +) + type ReleaserPleaser struct { forge forge.Forge logger *slog.Logger @@ -57,7 +66,7 @@ func (rp *ReleaserPleaser) Run(ctx context.Context) error { return fmt.Errorf("failed to create pending releases: %w", err) } - err = rp.runReconcileReleasePR(ctx) + err = rp.runReconcileReleasePRWithRetries(ctx) if err != nil { return fmt.Errorf("failed to reconcile release pull request: %w", err) } @@ -143,6 +152,36 @@ func (rp *ReleaserPleaser) createPendingRelease(ctx context.Context, pr *release return nil } +// runReconcileReleasePRWithRetries retries runReconcileReleasePR up to PullRequestConflictAttempts times, but only +// when a ErrorPullRequestConflict was encountered. +func (rp *ReleaserPleaser) runReconcileReleasePRWithRetries(ctx context.Context) error { + logger := rp.logger.With("method", "runReconcileReleasePRWithRetries", "totalAttempts", PullRequestConflictAttempts) + var err error + + for i := range PullRequestConflictAttempts { + logger := logger.With("attempt", i+1) + logger.DebugContext(ctx, "attempting runReconcileReleasePR") + + err = rp.runReconcileReleasePR(ctx) + if err != nil { + if errors.Is(err, ErrorPullRequestConflict) { + logger.WarnContext(ctx, "detected conflict while updating pull request description, retrying") + continue + } + + break + } + + break + } + + if err != nil { + return err + } + + return nil +} + func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error { logger := rp.logger.With("method", "runReconcileReleasePR") @@ -305,6 +344,23 @@ func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error { } logger.InfoContext(ctx, "opened pull request", "pr.title", pr.Title, "pr.id", pr.ID, "pr.url", rp.forge.PullRequestURL(pr.ID)) } else { + // Check if the pull request was updated while releaser-pleaser was running. + // This avoids a conflict where the user updated the PR while releaser-pleaser already pulled the info, and + // releaser-pleaser subsequently reverts the users changes. There is still a minimal time window for this to + // happen between us checking the PR again and submitting our changes. + + logger.DebugContext(ctx, "checking for conflict in pr description", "pr.id", pr.ID) + recheckPR, err := rp.forge.PullRequestForBranch(ctx, rpBranch) + if err != nil { + return err + } + if recheckPR == nil { + return fmt.Errorf("PR was deleted while releaser-pleaser was running") + } + if recheckPR.Description != pr.Description { + return ErrorPullRequestConflict + } + pr.SetTitle(rp.targetBranch, nextVersion) overrides, err := pr.GetOverrides()