Repository navigation
fix(forge): stop a default-branch change from landing on a branch a push is deleting - #331
Merged
Merged
Conversation
…ing (#316) The pre-receive hook reads the default branch when a push starts, so a PATCH default_branch that ran while a push deleting the new default was in flight left HEAD and the row naming a missing branch. The PATCH now takes a lease from the transport registry: it waits up to 10 s until no writer (push, maintenance) is registered, and commits the row only if no writer registered since, otherwise it restores HEAD and answers 503. e2e_default_race holds a real push in its hook (test-only SC_FORGE_HOOK_HOLD) between its read of the row and its ref update; without the fix the PATCH answers 200 and the default ends up naming the deleted branch.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
… on any failed commit From review: a PATCH that resends the current default_branch no longer waits for (or 503s behind) pushes; HEAD is put back on a failed row write as well as on a lost lease, with a unit test for the restore; the hook hold knob is compiled into debug builds only; the lock-order note names the registry mutex as innermost.
tonibergholm
approved these changes
Nov 9, 2026
5 of 8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A
PATCH default_branchsent while a push was deleting the new default could leave the repository's default naming a branch that no longer exists. The push'spre-receivehook had read the old default when it started, so it let the delete through. This was the first item on #316.The PATCH now takes a lease from the transport registry:
this repository is being written to, try again.HEADback and answers 503. A push that registers after the commit reads the new default in its hook.forgetstill drops a slot along with its repository.e2e_default_race.rsis the regression test. It holds a realgit push :devin its hook after the hook has read the row, usingSC_FORGE_HOOK_HOLD, a test-only knob read from the server's own environment, likeSC_FORGE_MERGE_PAUSE_MS. While the push is held, it sends the PATCH.Proof: without the
lifecycle.rschange, the PATCH answers 200 and the default then names the deleteddev; with it, the PATCH answers 503 and the default staysmain. fmt, clippy (-D warnings) andcargo test -p scl-forgepass locally.After an Opus review: a PATCH that resends the current default returns at once and doesn't wait for pushes (clients that send the whole object do this),
HEADis also put back when the row write fails, a unit test covers the restore, and the hold knob and its test exist in debug builds only.Trade-off: a real default-branch change answers 503 while a push or maintenance pass runs on that repository for longer than 10 s. Gitea doesn't do this.
Not covered: branch-protection rule changes have the same kind of race with a push that is already running. That one stays documented and out of scope.
Reviewed with sol-loop: 2 rounds with gpt-6.1-sol on high. Sol verdict: approved
Created with Claude Opus 5.5 in T3 Code (Claude Code harness).