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
This commit is contained in:
Julian Tölle 2025-06-14 15:23:05 +02:00 committed by GitHub
parent 08d35f2f57
commit d24ae7de98
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -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()