Skip to content

Let the merge sync lose a push race instead of failing - #23

Merged
darksidemilk merged 1 commit into
mainfrom
gh-1219-fix-merge-sync-push-race
Aug 20, 2026
Merged

Let the merge sync lose a push race instead of failing#23
darksidemilk merged 1 commit into
mainfrom
gh-1219-fix-merge-sync-push-race

Conversation

@darksidemilk

Copy link
Copy Markdown
Member

Closes FOGProject/fogproject#1219

sync-generated-files.yml in fogproject is disabled right now because this step keeps
failing. It is a push race, not a permissions problem — worth saying, because the previous two
failures on this path were permissions and that is where the eye goes.

What happens

The job checks out a branch, then spends ~50s on checkout, apt-get, the php-cs-fixer download,
translation regen and PSR2 formatting before it pushes. Any PR merged into that branch during the
window moves the remote:

! [rejected]  HEAD -> working-1.6 (fetch first)

Measured on the run that took the stub down:

Time Event
14:11:43 actions/checkout of working-1.6
14:12:05 an unrelated PR is merged into working-1.6
14:12:32 git push → rejected

3 of the last 15 runs died this way, each within ~60s of another run.

Not a race between two sync runs. The stub's concurrency group already serialises those. This
is the job racing a human pressing Merge, which no concurrency setting reaches — and cross-repo
concurrency does not exist anyway, since groups are per-repository.

Why fail soft rather than rebase and retry

FOG_VERSION is derived from commit count since master. The instant the remote gains a commit,
the version computed a few steps earlier is stale — a rebase would push a number that is already
wrong. Skipping keeps correctness because the next merge sync or the daily cron recomputes from
scratch, and it cannot feed itself, which matters more here than anywhere else in this repo given
the 30-commits-in-20-minutes incident a self-triggering sync once caused.

Same trade the stub already documents for fork PRs: better a gap the schedule already covers than
a red X.

The check is specific, not blanket. HEAD~1 is the commit the job checked out, since the commit it
makes is the only one. If origin/<branch> is no longer that, the branch moved and skipping is
right; if it still is, the push failed for another reason and the job still fails.

The second edit is not optional

Summarize keys its version line on steps.version.outcome — the "Apply version fix" step,
which succeeds whether or not anything reached the remote. So a skipped push would still have
printed "🔧 Version fixed — old → new" for a version that was never pushed, while the
steps.commit.outcome check below stayed quiet because the step now exits 0.

Fixing only the push would have traded a red X for a false report. The raced branch is emitted
before the version line so it wins, and steps.commit.outcome now covers only genuine failures.

Verification

The race is impractical to wait for, so it was reproduced with real git — three scenarios against
a real bare remote:

GH-1219 push logic
  ok    raced: exit status (0)
  ok    raced: emits ::warning:: (yes)
  ok    raced: sets raced=true output (yes)
  ok    raced: remote untouched
  ok    genuine failure: non-zero exit (yes)
  ok    genuine failure: does NOT set raced (unset)
  ok    clean push: exit status (0)
  ok    clean push: remote advanced

8 passed, 0 failed

After merge

Re-enabling fogproject's workflow is a repository setting, not a diff, so it does not arrive with
this PR:

gh workflow enable 336670600 --repo FOGProject/fogproject

🤖 Generated with Claude Code

https://claude.ai/code/session_01PqufBbuckux8kitJeW3uAK

The sync job checks out a branch and then spends about fifty seconds on
checkout, apt-get, the php-cs-fixer download, translation regeneration and PSR2
formatting before it pushes. Any PR merged into that branch during the window
moves the remote and the push is rejected:

    ! [rejected]  HEAD -> working-1.6 (fetch first)

Measured on the run that took fogproject's merge stub down: checkout at
14:11:43, push at 14:12:32, and an unrelated PR merged at 14:12:05. Three of the
last fifteen runs died this way, each within a minute of another run, and the
stub is disabled because of it.

Not a race between two sync runs. The stub's concurrency group already
serialises those. It is a race between this job and a human pressing Merge, and
no concurrency setting reaches that -- cross-repo concurrency does not exist
either, since groups are per-repository, so fogproject's stub and this repo's
cron could never share one regardless.

Failing soft rather than rebasing and retrying, deliberately. FOG_VERSION is
derived from commit count since master, so the instant the remote gains a commit
the version computed a few steps earlier is stale; a rebase would push a number
that is already wrong. Skipping keeps correctness, because the next merge sync
or the daily cron recomputes from scratch, and it cannot feed itself -- which
matters more on this path than anywhere else in the repository, given the 30
commits in 20 minutes that a self-triggering sync once put on dev-branch. It is
the same trade the stub already documents for fork PRs: better a gap the
schedule already covers than a red X.

The check is specific rather than blanket. HEAD~1 is the commit the job checked
out, because the commit it makes is the only one; if origin/<branch> is no
longer that commit the branch moved and the skip is right, and if it IS still
that commit the push failed for some other reason and the job still fails.

Summarize needed the second half of this. It keys the version line on
steps.version.outcome, which is the "Apply version fix" step and succeeds
whether or not anything reached the remote -- so a skipped push would have
reported "Version fixed" for a version that was never pushed, while the
steps.commit.outcome check below it stayed quiet because the step now exits 0.
Fixing only the push would have traded a red X for a false report, which is
worse than the red X. The raced branch is emitted before the version line so it
wins, and steps.commit.outcome now covers only genuine failures, which is what
it was for.

Verified by construction with real git rather than by waiting for the race:
another clone pushes a commit between the checkout and the push (skip, warning,
remote untouched), an unreachable remote with the branch unmoved (still fails),
and the undisturbed case (still pushes).

Turning fogproject's workflow back on is a repository setting and does not
arrive with this commit:

    gh workflow enable 336670600 --repo FOGProject/fogproject

Closes FOGProject/fogproject#1219

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqufBbuckux8kitJeW3uAK
@darksidemilk
darksidemilk merged commit 6ff022f into main Aug 20, 2026
@darksidemilk
darksidemilk deleted the gh-1219-fix-merge-sync-push-race branch August 20, 2026 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge sync fails when another PR is merged during its run

1 participant