Repository navigation
fix(releasing): a plain push past a journaled head is not a force push - #202
Merged
Merged
Conversation
A Release Run that parked with a red pipeline journaled the head its awaited step had read. While parked, a plain push can move the branch past that head. On resume, release.workflow.ts replays fast-forward.<n> with the same journaled head, so fastForward saw a checkout that is not an ancestor of the requested head and refused it 'as after a force push' - and every resume replayed the same stale head, parking the Run forever. fastForward now distinguishes the two cases the old check collapsed: where the checkout already carries the requested head (it is an ancestor of the checkout, not the reverse) AND the checkout sits exactly at origin's branch tip, the journal simply holds a stale head, so answer At on what the checkout actually holds. A true rewrite - the checkout neither at nor behind the head, or sitting off origin's tip - is still refused, so nothing is reset or discarded. Tested with a regression pair: a plain push two commits past the journaled head is now taken At on its real tip (the case that used to park), and a true force push is still refused. Sabotage-run: the plain-push case fails on the old code, expected At / got Refused.
Contributor
Author
|
Fixed here: |
Collaborator
|
The replay exception avoids resetting a checkout that already carries the journaled head, and both new tests detect targeted breakage. However, its remote-tip guard accepts stale tracking refs after fetch failure or branch deletion, so it cannot establish that the checkout still matches origin. Blocker
Minor
|
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.
Problem
A Release Run that parked on a red pipeline journals the head its
awaitedstep had read. While the Run is parked, a plain push can move the branch past that journaled head (each commit descending from the last). On resume,release.workflow.tsreplaysfast-forward.<n>with the same journaled head, sofastForwardinsrc/releasing.tssees a checkout that is not an ancestor of the requested head and refuses it as "after a force push". Every resume replays the identical stale head, so the Run parks forever (#197'srun-23bd1febhad to be stopped and started fresh).Approach
fastForward'smerge-base --is-ancestor from headcheck collapsed two different situations. It now tells them apart before refusing:head(headis an ancestor of the checkout, the reverse direction) and the checkout sits exactly at origin's branch tip (refs/remotes/origin/<branch>), the Run's journal simply holds a stale head. This is a plain push, not a rewrite, so it answersAton what the checkout actually holds, and the nextawaitedread sees the real head and re-proves it.This is the behavior the issue's "Expected" section asks for: answer
Atand let the next read seeMovedto the real head, refusing only a genuine rewrite.Tests
Two new regression tests in
test/releasing.test.ts, using the file's existing bare-origin and real-git harness:fastForwardon the stale head now returnsAton the real tip (the case that used to park).mainis rewritten in place so the checkout no longer carries the journaled head; stillRefusedwith the "force push" reason.Sabotage-run: with the fix reverted, the plain-push test fails (expected
At, gotRefused) while the force-push test passes, so the first test bites and the second guards against over-refusing.bun test test/releasing.test.tsis green (8 pass), oxlint and oxfmt are clean on both files, andtsc --noEmitreports the same 11 pre-existing errors with and without the change (all indesktop/andtest/flock-chat-window.test.ts, none in the changed files).Risk
The new branch is taken only when the checkout is clean, already carries the requested head, and sits exactly at origin's branch tip: the narrow case where the journal is provably stale. The original refusal path is otherwise unchanged, so the "nothing is reset or discarded" guarantee holds.
Fixes #200