Skip to content

fix(releasing): a plain push past a journaled head is not a force push - #202

Merged
madskronborg merged 1 commit into
cego:masterfrom
XRafik:fix/fastforward-stale-head
Oct 26, 2026
Merged

madskronborg merged 1 commit into
cego:masterfrom
XRafik:fix/fastforward-stale-head

Conversation

@XRafik

@XRafik XRafik commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Problem

A Release Run that parked on a red pipeline journals the head its awaited step 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.ts replays fast-forward.<n> with the same journaled head, so fastForward in src/releasing.ts sees 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's run-23bd1feb had to be stopped and started fresh).

Approach

fastForward's merge-base --is-ancestor from head check collapsed two different situations. It now tells them apart before refusing:

  • Where the checkout already carries the requested head (head is 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 answers At on what the checkout actually holds, and the next awaited read sees the real head and re-proves it.
  • A true force push (the checkout neither at nor behind the requested head, or sitting off origin's tip) is still refused, so nothing is reset or discarded.

This is the behavior the issue's "Expected" section asks for: answer At and let the next read see Moved to 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:

  • plain push past the journaled head: two commits descend from the journaled head and the checkout sits at the tip; fastForward on the stale head now returns At on the real tip (the case that used to park).
  • true force push: main is rewritten in place so the checkout no longer carries the journaled head; still Refused with the "force push" reason.

Sabotage-run: with the fix reverted, the plain-push test fails (expected At, got Refused) while the force-push test passes, so the first test bites and the second guards against over-refusing. bun test test/releasing.test.ts is green (8 pass), oxlint and oxfmt are clean on both files, and tsc --noEmit reports the same 11 pre-existing errors with and without the change (all in desktop/ and test/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

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.
@XRafik

XRafik commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Fixed here: fastForward now tells a plain push that moved the branch past the journaled head (while the Run parked) apart from a true force push, so a resumed Run re-proves the real head instead of parking on the stale one. Added a regression pair (plain push taken At; true force push still refused) and ran the sabotage check.

@madskronborg

Copy link
Copy Markdown
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

  • src/releasing.ts:59 — Confirm the current remote tip before accepting a stale head
    Correctness axis: after C was pushed, origin can be force-rewound to ancestor A and a failed fetch leaves origin/ cached at C, causing this path to return At(C) for A; deleting the remote branch also leaves this guard true after a successful fetch without pruning. This violates the exception's origin-tip condition and can repeatedly prove the wrong head instead of parking on a rewrite; require a successful current remote-tip observation rather than the cached ref alone (the sole new acceptance path).

Minor

  • src/releasing.ts:48 — Keep only the essential replay rationale in comments
    Readability and standards axes, code-comment hygiene: src/releasing.ts:48-53 repeats the adjacent API explanation at length, while test/releasing.test.ts:82-83, 88-90 and 104-106 narrate the fixtures and repeat the replay explanation. Retain the essential journal-replay and loose-object reasons in short sentences, removing the repeated narration.

@madskronborg
madskronborg merged commit 190b4ae into cego:master Oct 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

A parked release cannot resume after its branch moved past the head it last saw

2 participants