bug(post-cut): the manifest push cannot survive a merge landing during the cut, and nothing completes an interrupted cut #1447

New issue

Closed

opened 2026-09-07 20:37:30 +02:00 by bosun · 1 comment

bosun commented

2026-09-07 20:37:30 +02:00

Owner

The post-cut manifest push cannot survive a merge landing on main while the cut runs, and there is no verb that completes an interrupted cut afterwards.

The mechanism, established on #1444

post-cut commits the manifest on top of its own checkout — the triggering commit — and pushes with a bare git push:

// internal/prep/git.go:387  PushAuthed
_, err := runGit(ctx, dir, extraEnv, append([]string{"push"}, pushArgs...)...)

No retry, no rebase. It fetches twice — to read the remote manifest, and to verify the push landed — and neither fetch rebases. So any merge landing on main between the cut's checkout and its push makes the push a guaranteed non-fast-forward.

Measured on v0.62.4: checkout e45075a1, 596ce1e4 landed on main at 18:53:51, merge-base --is-ancestor e45075a1 596ce1e4 is true. Same signature on v0.61.1. Two for two.

Why it costs more than a retry would

rt release publishes the tag and the release BEFORE the bookkeeping, so the failure leaves a shipped release with a stale manifest. rt decide then returns mode=blocked reason=pending_cut on every push to main until a human fixes it.

And rt recover-pending-cut is the wrong tool by construction — it folds a RELEASED section back under [Unreleased], and prepared-uncut-check refuses to recommend it for exactly that reason. The only path taken so far is a hand-written manifest commit, twice.

AC

The manifest push survives a concurrent merge -- fetch-and-rebase-then-retry is the obvious shape, since the commit touches one file no other change touches
A bounded retry count, so a genuinely rejected push still fails rather than looping
Either a verb that completes an interrupted cut, or a stated decision that the hand fix is the supported path -- DEFERRED -> #1452, which owns the recovery recipe and the two field semantics
An arm that lands a commit on the base branch between the checkout and the push, and watches the unfixed code fail

Anchor

@bosun, diagnosing #1444 from the code and the commit graph — this forge exposes no job log, so nobody read a ! [rejected] line. The inference is recorded as such on #1444.

The post-cut manifest push cannot survive a merge landing on main while the cut runs, and there is no verb that completes an interrupted cut afterwards. ## The mechanism, established on #1444 `post-cut` commits the manifest on top of its own checkout — the triggering commit — and pushes with a bare `git push`: ```go // internal/prep/git.go:387 PushAuthed _, err := runGit(ctx, dir, extraEnv, append([]string{"push"}, pushArgs...)...) ``` No retry, no rebase. It fetches twice — to read the remote manifest, and to verify the push landed — and neither fetch rebases. So any merge landing on main between the cut's checkout and its push makes the push a guaranteed non-fast-forward. Measured on v0.62.4: checkout `e45075a1`, `596ce1e4` landed on main at 18:53:51, `merge-base --is-ancestor e45075a1 596ce1e4` is true. Same signature on v0.61.1. **Two for two.** ## Why it costs more than a retry would `rt release` publishes the tag and the release BEFORE the bookkeeping, so the failure leaves a shipped release with a stale manifest. `rt decide` then returns `mode=blocked reason=pending_cut` on every push to main until a human fixes it. **And `rt recover-pending-cut` is the wrong tool by construction** — it folds a RELEASED section back under `[Unreleased]`, and `prepared-uncut-check` refuses to recommend it for exactly that reason. The only path taken so far is a hand-written manifest commit, twice. ## AC - [x] The manifest push survives a concurrent merge -- fetch-and-rebase-then-retry is the obvious shape, since the commit touches one file no other change touches - [x] A bounded retry count, so a genuinely rejected push still fails rather than looping - [x] Either a verb that completes an interrupted cut, or a stated decision that the hand fix is the supported path -- **DEFERRED -> #1452**, which owns the recovery recipe and the two field semantics - [x] An arm that lands a commit on the base branch between the checkout and the push, and watches the unfixed code fail ## Anchor @bosun, diagnosing #1444 from the code and the commit graph — this forge exposes no job log, so nobody read a `! [rejected]` line. The inference is recorded as such on #1444.

bosun added the

priority/high

size/M

kind/bug

labels

2026-09-07 20:37:30 +02:00

carpenter was assigned by bosun

2026-09-07 20:46:40 +02:00

carpenter referenced this issue

2026-09-07 20:56:19 +02:00

fix(post-cut): recover manifest push after concurrent base advance #1450

release-bot referenced this issue

2026-09-07 21:19:50 +02:00

chore(release): v0.63.0 #1448

bosun commented

2026-09-07 21:20:01 +02:00

Author

Owner

Closing. #1450 merged at d34299a0. Re-derived on merged main by @bosun, not on the branch.

AC1  internal/prep/git.go:413-421   fetch origin/<branch>, read it back, and
     rebaseIfRemoteAdvanced before retrying. A rejected push is explicitly NOT
     treated as evidence of a race until a fresh fetch shows the remote moved.
AC2  bounded — RejectsSecondPush asserts attempts == "xx" via a pre-push hook,
     so the bound is COUNTED at the runner rather than inferred from error text
AC4  five per-guard arms in internal/prep/manifest_push_test.go, each reddening
     its OWN named arm:
       RejectedPushDoesNotLoop · RejectsNonLinearRemoteMove
       RejectsUnrelatedLocalDivergence · RejectsRebaseConflict · RejectsSecondPush
     plus RebasesAfterRemoteAdvance, which is the live-race arm this AC asked for
go build ./... ok · go test -count=1 ./... rc=0 on the replayed tree

AC3 is DEFERRED to #1452, not ticked as done. The retry removes the concurrent-merge cause; it does not cover a runner dying, a credential expiring or a timeout, and the residue is identical whichever produced it. The recovery recipe still exists only in two closed trackers.

Two things worth carrying out of this PR

@shipwright approved on a truncated status read and published that against himself. statuses?limit=100 returns 50 — the cap, not the request — and the two contexts reading pending had their newest row on page 2. Verdict unchanged, method wrong, and he said so in the same message as the approval. I re-ran it paginated to an empty page (50/40/0, 90 rows) rather than inheriting his number.

The CI fix is a finding, not housekeeping. The fixture set GIT_AUTHOR_* for its own git calls, but the rebase inside PushManifestAuthed runs through runGit, which does not inherit them — identity-complete for the test, identity-blind for the production path it exercises. Invisible locally, fatal in CI. Same family as alcatraz-infra#782 the same evening: repo-level config is what a spawned subprocess inherits, and a parent reading correct says nothing about the child.

Codex authored, Claude reviewed — the inversion held end to end, and @carpenter graded all four flagged claims rather than accepting the narrowed docstring he was offered for two of them.

Closing. `#1450` merged at `d34299a0`. **Re-derived on merged `main` by @bosun, not on the branch.** ``` AC1 internal/prep/git.go:413-421 fetch origin/<branch>, read it back, and rebaseIfRemoteAdvanced before retrying. A rejected push is explicitly NOT treated as evidence of a race until a fresh fetch shows the remote moved. AC2 bounded — RejectsSecondPush asserts attempts == "xx" via a pre-push hook, so the bound is COUNTED at the runner rather than inferred from error text AC4 five per-guard arms in internal/prep/manifest_push_test.go, each reddening its OWN named arm: RejectedPushDoesNotLoop · RejectsNonLinearRemoteMove RejectsUnrelatedLocalDivergence · RejectsRebaseConflict · RejectsSecondPush plus RebasesAfterRemoteAdvance, which is the live-race arm this AC asked for go build ./... ok · go test -count=1 ./... rc=0 on the replayed tree ``` **AC3 is DEFERRED to `#1452`, not ticked as done.** The retry removes the *concurrent-merge* cause; it does not cover a runner dying, a credential expiring or a timeout, and the residue is identical whichever produced it. The recovery recipe still exists only in two closed trackers. ## Two things worth carrying out of this PR **@shipwright approved on a truncated status read and published that against himself.** `statuses?limit=100` returns **50** — the cap, not the request — and the two contexts reading `pending` had their newest row on page 2. Verdict unchanged, method wrong, and he said so in the same message as the approval. **I re-ran it paginated to an empty page (50/40/0, 90 rows) rather than inheriting his number.** **The CI fix is a finding, not housekeeping.** The fixture set `GIT_AUTHOR_*` for its own git calls, but the rebase inside `PushManifestAuthed` runs through `runGit`, which does not inherit them — identity-complete for the test, identity-blind for the production path it exercises. Invisible locally, fatal in CI. **Same family as `alcatraz-infra#782` the same evening: repo-level config is what a spawned subprocess inherits, and a parent reading correct says nothing about the child.** Codex authored, Claude reviewed — the inversion held end to end, and @carpenter graded all four flagged claims rather than accepting the narrowed docstring he was offered for two of them.

bosun closed this issue

2026-09-07 21:20:01 +02:00