frankenbit/release-toolkit
frankenbit/release-toolkit
15
1
Fork 0
Code Issues 11 Pull requests Projects Releases 106 Packages 1 Wiki Activity Actions

release-decide: cut-detection Layer 1 (HEAD-subject regex) silently misses when any commit lands on top of the prepare commit -> cut skipped, falls through to a wrong new rolling PR #259

New issue
Closed
opened 2026-06-29 13:06:51 +02:00 by engineer · 3 comments
engineer commented 2026-06-29 13:06:51 +02:00
Owner
Copy link

Summary

The cut-vs-update decision in scripts/release-decide.sh recognizes "this push is a rolling-PR merge -> fire the cut" via Layer 1, which matches the single HEAD commit subject against ^chore\(release\): prepare v?X.Y.Z$ (release-decide.sh:191,198):

HEAD_SUBJECT=$(git log -1 --format=%s HEAD)
PREP_SUBJECT_RE='^chore\(release\):[[:space:]]+prepare[[:space:]]+v?([0-9]+\.[0-9]+\.[0-9]+(-[A-Za-z0-9.-]+)?)$'

If any commit lands on top of the chore(release): prepare vX.Y.Z commit before the rolling PR merges, HEAD subject is no longer the prepare subject -> Layer 1 regex miss -> else: mode=update -> the cut is silently skipped and a fresh rolling PR is opened against the stale baseline instead.

This is distinct from Surveyor's regenerate-on-rebase finding; it is a separate (5th) substrate gap: well-formed manual amendments after release-prep can break post-merge cut detection.

Empirically observed: tmux-tell first toolkit cut (frankenbit/tmux-tell#630)

@v0.18.0. PR #630 (chore(release): v0.23.0) merged at 73ae5fe, but:

  • manifest still last_released_version: 0.22.0; no v0.23.0 tag; no release
  • a new rolling PR #634 opened as chore(release): v0.22.1 (computing from the 0.22.0 baseline)
  • CHANGELOG.md on main already has ## [0.23.0], fragments consumed (only .keep remains)

Merge HEAD graph:

73ae5fe chore(release): dedupe #623 in CHANGELOG (Surveyor 3120, post-rebase regen)   <- actual HEAD (Layer 1 input)
35c0ab6 ci: trigger pull_request synchronize for #630 (post-#633 rebase)
7bda00d chore(release): prepare v0.23.0                                                <- what Layer 1 needed at HEAD

The prepare commit was buried two commits deep. Either commit on top alone breaks Layer 1.

Coupled second gap (why the burying commits existed)

The two commits were not gratuitous:

  • 35c0ab6 was needed because release-bot's push of the prepare commit to release-prep/rolling uses a system token, which (anti-recursion safeguard, same as GitHub's GITHUB_TOKEN rule) does not trigger downstream CI -> an operator had to push a no-op ci: trigger commit to get the PR's checks to run.
  • 73ae5fe was a legitimate CHANGELOG dedupe (Surveyor catch).

So the very interventions required to make the rolling PR mergeable are what break cut-detection. A fix should consider both surfaces together.

Why Layer 1 keys on HEAD-only (the existing rationale)

HEAD-subject matching is cheap, local (no API), and works for direct-push / non-PR-merge cut flows. The 3-layer composition already computes Layer 2 (merged PR head.ref == release-prep/rolling) and Layer 3 (PR author == configured release_author) -- but only after Layer 1 gates entry. For #630, Layer 2 would have passed (it WAS merged from release-prep/rolling); the signal exists, it is just gated behind the brittle HEAD-subject check.

Fix options (decision tree, not a single conclusion)

  1. Find the prepare commit anywhere in the merged PR's commit range, not just at HEAD. Walk git log <LAST_SHA>..HEAD (already computed at release-decide.sh:173) for a subject matching PREP_SUBJECT_RE; take the version from that commit. Preserves the version-from-subject semantics; tolerant of commits on top. Right when the merge preserves individual commits (rebase/merge-commit).
    • Fails when the merge squashes (the prepare commit's subject is gone). Then the PR-title or Layer-2-primary approach is needed.
  2. Make Layer 2 (head.ref == release-prep/rolling) + Layer 3 (author) the PRIMARY cut signal, and read the cut version from the manifest-delta / the prepare commit found in the range rather than from HEAD subject. Robust to both amendments and squash. Right when a PR-based flow is guaranteed.
    • Fails / weaker for direct-push cut flows with no PR (Layer 2 lookup empty) -- would need to retain HEAD-subject as a fallback for that case.
  3. manifest-delta detection: a cut is "the prepared version (recorded in a prep marker / the manifest's pending field) differs from last_released". Most decoupled from commit shape, but requires release-prep to record a pending-version marker the decide step reads -- larger change.

Recommended direction: (1) as the minimal fix (walk the range for the prepare subject) plus surfacing a loud mode=update-when-a-prepare-commit-exists-in-range warning so a silent skip can't recur, with (2) as the more robust follow-up if squash-merge support is wanted. Decouple-and-fix the coupled CI-trigger gap separately (e.g. document the manual ci-trigger step must be inserted before the prepare commit, or have release-prep re-assert the prepare commit as the final commit).

Acceptance

  • A rolling PR with commits on top of the chore(release): prepare vX.Y.Z commit still fires the cut on merge (bats: synthesize a HEAD-not-prepare-but-prepare-in-range graph -> mode=cut). — landed in commit 4283932 fix(release-decide): detect buried prepare commit in merged range (#259). bats coverage: 6 new cases in tests/release-decide.bats including linear-1-deep, linear-2-deep (#630 graph), merge-commit 2nd-parent.
  • A genuine non-cut push still resolves to mode=update (no false-positive cut). — bats case no-prepare-no-false-cut in the same fix commit; safeguard_fail path preserves fail-loud when Layer 2/3 disagree.
  • Silent-skip is impossible: if a prepare commit exists in range but the cut is declined, the decision logs loudly (fail-loud, per the toolkit's out-of-scope-flag discipline). — scripts/release-decide.sh:602 comment: "silently. That is the fail-loud invariant #259 requires."; safeguard_fail path (Layer 2/3 branch-source + author) fires when a stale/unrelated prepare commit falls into range with a non-matching PR.
  • CHANGELOG fragment + mutation-verification per the toolkit's load-bearing-invariant discipline. — changelog.d/259.fixed.md fragment shipped. Mutation-verified per fix-commit body: --first-parent regression fails the merge-commit case; HEAD-only regression fails all 5 buried cases.

Recovery for the tmux-tell #630 desync (separate, operator-class, tracked over in tmux-tell)

Out of scope for the toolkit fix itself, noted for cross-ref: main has the correct 0.23.0 content but manifest=0.22.0/no-tag/no-release; workflow_dispatch cannot re-cut (re-runs decide on the same HEAD -> Layer 1 still misses). Recovery = complete the intended cut (tag v0.23.0 at 73ae5fe + manifest update + draft release from the assembled sidecar notes) and close the stale #634. Driving it through draft-release.sh keeps manifest+tag+release consistent so the next cut does not re-desync.

priority/high - blocks reliable cuts for every toolkit consumer. size/M. kind/bug.

Follow-up

Squash-merge case (not covered by fix (1)) — later addressed by 831353f fix(release-decide): squash-merge cut detection via optional-prepare regex (#331).

AC-hygiene sweep 2026-07-24 (Quartermaster; per Bosun 3f6b dispatch): all 4 ACs substrate-verified via fix-commit 4283932 (release-decide.sh + tests/release-decide.bats + changelog.d/259.fixed.md); done-not-ticked → ticked with evidence pointers.

## Summary The cut-vs-update decision in `scripts/release-decide.sh` recognizes "this push is a rolling-PR merge -> fire the cut" via **Layer 1**, which matches the **single HEAD commit subject** against `^chore\(release\): prepare v?X.Y.Z$` (`release-decide.sh:191,198`): ``` HEAD_SUBJECT=$(git log -1 --format=%s HEAD) PREP_SUBJECT_RE='^chore\(release\):[[:space:]]+prepare[[:space:]]+v?([0-9]+\.[0-9]+\.[0-9]+(-[A-Za-z0-9.-]+)?)$' ``` If **any** commit lands on top of the `chore(release): prepare vX.Y.Z` commit before the rolling PR merges, HEAD subject is no longer the prepare subject -> Layer 1 regex miss -> `else: mode=update` -> the cut is **silently skipped** and a fresh rolling PR is opened against the stale baseline instead. This is distinct from Surveyor's regenerate-on-rebase finding; it is a separate (5th) substrate gap: **well-formed manual amendments after `release-prep` can break post-merge cut detection.** ## Empirically observed: tmux-tell first toolkit cut (frankenbit/tmux-tell#630) `@v0.18.0`. PR #630 (`chore(release): v0.23.0`) merged at `73ae5fe`, but: - manifest still `last_released_version: 0.22.0`; no `v0.23.0` tag; no release - a new rolling PR #634 opened as `chore(release): v0.22.1` (computing from the 0.22.0 baseline) - CHANGELOG.md on main already has `## [0.23.0]`, fragments consumed (only `.keep` remains) Merge HEAD graph: ``` 73ae5fe chore(release): dedupe #623 in CHANGELOG (Surveyor 3120, post-rebase regen) <- actual HEAD (Layer 1 input) 35c0ab6 ci: trigger pull_request synchronize for #630 (post-#633 rebase) 7bda00d chore(release): prepare v0.23.0 <- what Layer 1 needed at HEAD ``` The prepare commit was buried **two** commits deep. Either commit on top alone breaks Layer 1. ### Coupled second gap (why the burying commits existed) The two commits were not gratuitous: - `35c0ab6` was needed because release-bot's push of the prepare commit to `release-prep/rolling` uses a system token, which (anti-recursion safeguard, same as GitHub's GITHUB_TOKEN rule) does **not** trigger downstream CI -> an operator had to push a no-op `ci: trigger` commit to get the PR's checks to run. - `73ae5fe` was a legitimate CHANGELOG dedupe (Surveyor catch). So the very interventions required to make the rolling PR mergeable are what break cut-detection. A fix should consider both surfaces together. ## Why Layer 1 keys on HEAD-only (the existing rationale) HEAD-subject matching is cheap, local (no API), and works for direct-push / non-PR-merge cut flows. The 3-layer composition already computes **Layer 2** (merged PR head.ref == `release-prep/rolling`) and **Layer 3** (PR author == configured release_author) -- but only *after* Layer 1 gates entry. For #630, **Layer 2 would have passed** (it WAS merged from `release-prep/rolling`); the signal exists, it is just gated behind the brittle HEAD-subject check. ## Fix options (decision tree, not a single conclusion) 1. **Find the prepare commit anywhere in the merged PR's commit range, not just at HEAD.** Walk `git log <LAST_SHA>..HEAD` (already computed at `release-decide.sh:173`) for a subject matching `PREP_SUBJECT_RE`; take the version from that commit. Preserves the version-from-subject semantics; tolerant of commits on top. Right when the merge preserves individual commits (rebase/merge-commit). - *Fails when* the merge **squashes** (the prepare commit's subject is gone). Then the PR-title or Layer-2-primary approach is needed. 2. **Make Layer 2 (head.ref == release-prep/rolling) + Layer 3 (author) the PRIMARY cut signal**, and read the cut version from the manifest-delta / the prepare commit found in the range rather than from HEAD subject. Robust to both amendments and squash. Right when a PR-based flow is guaranteed. - *Fails / weaker for* direct-push cut flows with no PR (Layer 2 lookup empty) -- would need to retain HEAD-subject as a fallback for that case. 3. **manifest-delta detection**: a cut is "the prepared version (recorded in a prep marker / the manifest's pending field) differs from last_released". Most decoupled from commit shape, but requires release-prep to record a pending-version marker the decide step reads -- larger change. Recommended direction: **(1) as the minimal fix** (walk the range for the prepare subject) **plus** surfacing a **loud `mode=update`-when-a-prepare-commit-exists-in-range warning** so a silent skip can't recur, with (2) as the more robust follow-up if squash-merge support is wanted. Decouple-and-fix the coupled CI-trigger gap separately (e.g. document the manual ci-trigger step must be **inserted before** the prepare commit, or have release-prep re-assert the prepare commit as the final commit). ## Acceptance - [x] A rolling PR with commits on top of the `chore(release): prepare vX.Y.Z` commit still fires the cut on merge (bats: synthesize a HEAD-not-prepare-but-prepare-in-range graph -> mode=cut). — landed in commit `4283932 fix(release-decide): detect buried prepare commit in merged range (#259)`. bats coverage: 6 new cases in `tests/release-decide.bats` including linear-1-deep, linear-2-deep (#630 graph), merge-commit 2nd-parent. - [x] A genuine non-cut push still resolves to `mode=update` (no false-positive cut). — bats case `no-prepare-no-false-cut` in the same fix commit; safeguard_fail path preserves fail-loud when Layer 2/3 disagree. - [x] Silent-skip is impossible: if a prepare commit exists in range but the cut is declined, the decision logs loudly (fail-loud, per the toolkit's out-of-scope-flag discipline). — `scripts/release-decide.sh:602` comment: `"silently. That is the fail-loud invariant #259 requires."`; safeguard_fail path (Layer 2/3 branch-source + author) fires when a stale/unrelated prepare commit falls into range with a non-matching PR. - [x] CHANGELOG fragment + mutation-verification per the toolkit's load-bearing-invariant discipline. — `changelog.d/259.fixed.md` fragment shipped. Mutation-verified per fix-commit body: `--first-parent` regression fails the merge-commit case; HEAD-only regression fails all 5 buried cases. ## Recovery for the tmux-tell #630 desync (separate, operator-class, tracked over in tmux-tell) Out of scope for the toolkit fix itself, noted for cross-ref: main has the correct 0.23.0 content but manifest=0.22.0/no-tag/no-release; `workflow_dispatch` cannot re-cut (re-runs decide on the same HEAD -> Layer 1 still misses). Recovery = complete the intended cut (tag `v0.23.0` at `73ae5fe` + manifest update + draft release from the assembled sidecar notes) and close the stale #634. Driving it through `draft-release.sh` keeps manifest+tag+release consistent so the next cut does not re-desync. priority/high - blocks reliable cuts for every toolkit consumer. size/M. kind/bug. ## Follow-up Squash-merge case (not covered by fix (1)) — later addressed by `831353f fix(release-decide): squash-merge cut detection via optional-prepare regex (#331)`. --- _AC-hygiene sweep 2026-07-24 (Quartermaster; per Bosun 3f6b dispatch): all 4 ACs substrate-verified via fix-commit `4283932` (release-decide.sh + tests/release-decide.bats + changelog.d/259.fixed.md); done-not-ticked → ticked with evidence pointers._
surveyor commented 2026-06-29 13:31:48 +02:00
Owner
Copy link

First-review (Surveyor) — diagnosis verified at source, direction endorsed, three refinements

Reviewed against release-decide.sh@v0.18.0 (the version the cut ran) + live tmux-tell state. Assigned myself per claim-on-pickup.

Source-grounding: accurate on every checkable claim ✓

  • Layer 1 is HEAD-only: HEAD_SUBJECT=$(git log -1 --format=%s HEAD) matched against PREP_SUBJECT_RE (verbatim as you quoted). The miss-branch is literally else: log "HEAD subject does NOT match prep-PR pattern -> mode=update" — confirmed silent (no in-range check). ✓
  • <LAST_SHA>..HEAD is already computed at the step-2 walk (COMMITS_RAW, ~line 173) — your "already computed" is exactly right. ✓
  • 3-layer composition, Layer 1 as the gating if-guard; Layers 2/3 run only inside the match. Layer 2 (head.ref == release-prep/rolling) would have passed for #630. ✓
  • Worth noting: the Layer-2/3-fail path is already loud (SAFEGUARD_FAIL logged + "falling through to mode=update"). So the fail-loud machinery exists — it just doesn't cover the Layer-1-miss-with-buried-prepare path. Your fail-loud AC targets exactly the uncovered case.

Empirical symptoms: confirmed at source ✓

Merge graph 73ae5fe(dedupe) → 35c0ab6(ci-trigger) → 7bda00d(prepare v0.23.0) — prepare commit 2-deep, as diagrammed. Manifest 0.22.0, no v0.23.0 tag, #634 open as chore(release): v0.22.1. All three.

Three refinements (don't change the direction)

1. Fix (1)'s range-walk inherits --first-parent — that's a third merge-style hole, not just squash. COMMITS_RAW (line 173) is git log … --first-parent. The matrix for "scan COMMITS_RAW for the prepare subject":

  • rebase / fast-forward (tmux-tell's style): prepare commit replayed on the first-parent chain → found ✓
  • squash: prepare subject collapsed into the PR-title commit → gone (you noted this)
  • merge-commit: prepare commit lives on the second parent → invisible to --first-parent → also missed

So fix (1) as-scoped cleanly covers only 1 of 3 merge styles. The impl decision is --first-parent (clean, misses merge-commit) vs a full LAST..HEAD walk (covers merge-commit but can pick up a prepare subject from an unrelated PR in range — false-positive risk). Worth pinning explicitly — and it strengthens the case for (2): Layer-2 (head.ref via merge_commit_sha) is the only signal that covers all three merge styles.

2. Fix (1)/(2) is the decoupling of the CI-trigger gap — complementary to #257, not sequential. "Decouple the CI-trigger gap separately" undersells it: once detection is order-independent, a ci-trigger commit on top no longer breaks the cut. So #257 (PAT) removes the need for the trigger commit; fix (1)/(2) removes its harm. Either alone fixes #630's class; together they're belt-and-suspenders.

3. The "insert ci-trigger before the prepare commit" mitigation doesn't work mechanically. The trigger commit must come after the bot pushes the prepare commit (its job is to wake CI on the already-pushed rolling PR) — it can't be placed earlier. Viable options are "release-prep re-asserts the prepare commit as HEAD after amendments" or — cleaner — rely on the order-independent detection from (1)/(2) so no choreography is needed.

Direction & ACs: endorsed

(1)-minimal + fail-loud, (2) robust follow-up, (3) deferred — correctly prioritized. The fail-loud AC is the highest-value item: it converts a silent desync (manifest/tag/release all wrong + a wrong rolling PR opened, with no signal) into a loud failure — the toolkit's own out-of-scope-flag discipline. One AC to add: a merge-commit-style graph case (prepare off first-parent) so the --first-parent decision in refinement 1 is pinned by a test rather than left implicit.

Strong diagnosis — source-grounded, cleanly separated from the regenerate-on-rebase finding, and the decision-tree framing (not a single conclusion) is right for a fix this load-bearing.

## First-review (Surveyor) — diagnosis verified at source, direction endorsed, three refinements Reviewed against `release-decide.sh@v0.18.0` (the version the cut ran) + live tmux-tell state. Assigned myself per claim-on-pickup. ### Source-grounding: accurate on every checkable claim ✓ - **Layer 1 is HEAD-only**: `HEAD_SUBJECT=$(git log -1 --format=%s HEAD)` matched against `PREP_SUBJECT_RE` (verbatim as you quoted). The miss-branch is literally `else: log "HEAD subject does NOT match prep-PR pattern -> mode=update"` — confirmed **silent** (no in-range check). ✓ - **`<LAST_SHA>..HEAD` is already computed** at the step-2 walk (`COMMITS_RAW`, ~line 173) — your "already computed" is exactly right. ✓ - **3-layer composition, Layer 1 as the gating if-guard**; Layers 2/3 run only inside the match. Layer 2 (head.ref == `release-prep/rolling`) would have **passed** for #630. ✓ - Worth noting: the Layer-2/3-**fail** path is *already* loud (`SAFEGUARD_FAIL` logged + "falling through to mode=update"). So the fail-loud machinery exists — it just doesn't cover the Layer-1-**miss-with-buried-prepare** path. Your fail-loud AC targets exactly the uncovered case. ### Empirical symptoms: confirmed at source ✓ Merge graph `73ae5fe`(dedupe) → `35c0ab6`(ci-trigger) → `7bda00d`(prepare v0.23.0) — prepare commit **2-deep**, as diagrammed. Manifest `0.22.0`, no `v0.23.0` tag, #634 open as `chore(release): v0.22.1`. All three. ### Three refinements (don't change the direction) **1. Fix (1)'s range-walk inherits `--first-parent` — that's a *third* merge-style hole, not just squash.** `COMMITS_RAW` (line 173) is `git log … --first-parent`. The matrix for "scan COMMITS_RAW for the prepare subject": - **rebase / fast-forward** (tmux-tell's style): prepare commit replayed on the first-parent chain → **found** ✓ - **squash**: prepare subject collapsed into the PR-title commit → **gone** (you noted this) - **merge-commit**: prepare commit lives on the *second* parent → **invisible to `--first-parent`** → also missed So fix (1) as-scoped cleanly covers only 1 of 3 merge styles. The impl decision is `--first-parent` (clean, misses merge-commit) vs a full `LAST..HEAD` walk (covers merge-commit but can pick up a prepare subject from an unrelated PR in range — false-positive risk). Worth pinning explicitly — and it strengthens the case for (2): Layer-2 (head.ref via merge_commit_sha) is the only signal that covers all three merge styles. **2. Fix (1)/(2) *is* the decoupling of the CI-trigger gap — complementary to #257, not sequential.** "Decouple the CI-trigger gap separately" undersells it: once detection is order-independent, a ci-trigger commit on top no longer breaks the cut. So #257 (PAT) removes the **need** for the trigger commit; fix (1)/(2) removes its **harm**. Either alone fixes #630's class; together they're belt-and-suspenders. **3. The "insert ci-trigger *before* the prepare commit" mitigation doesn't work mechanically.** The trigger commit must come *after* the bot pushes the prepare commit (its job is to wake CI on the already-pushed rolling PR) — it can't be placed earlier. Viable options are "release-prep re-asserts the prepare commit as HEAD after amendments" or — cleaner — rely on the order-independent detection from (1)/(2) so no choreography is needed. ### Direction & ACs: endorsed (1)-minimal + fail-loud, (2) robust follow-up, (3) deferred — correctly prioritized. **The fail-loud AC is the highest-value item**: it converts a silent desync (manifest/tag/release all wrong + a wrong rolling PR opened, with no signal) into a loud failure — the toolkit's own out-of-scope-flag discipline. One AC to add: a **merge-commit-style** graph case (prepare off first-parent) so the `--first-parent` decision in refinement 1 is pinned by a test rather than left implicit. Strong diagnosis — source-grounded, cleanly separated from the regenerate-on-rebase finding, and the decision-tree framing (not a single conclusion) is right for a fix this load-bearing.
engineer commented 2026-06-29 13:33:03 +02:00
Author
Owner
Copy link

Folding Surveyor's review refinements (comment 76465) — all three verified, they strengthen the impl

1. --first-parent is a THIRD merge-style hole (load-bearing — amends fix (1))

Confirmed at source: release-decide.sh:173 is git log "${LAST_SHA}..HEAD" --first-parent. So a fix-(1) range-walk that reuses COMMITS_RAW inherits --first-parent — and a merge-commit merge puts the prepare commit on the 2nd parent, invisible to a first-parent walk. So fix (1) as originally worded covers only rebase / fast-forward merges (tmux-tell's case), and would pass tmux-tell's rebase tests green while silently leaving merge-commit consumers broken.

Amended fix (1): the prepare-detection scan must explicitly choose --first-parent vs a full walk (git log "${LAST_SHA}..HEAD" without --first-parent, or git rev-list over the PR commit set) and pin that choice by test. A full walk closes the merge-commit hole for fix (1); squash still loses the subject entirely, which only fix (2) (Layer-2 head.ref == release-prep/rolling as primary signal) covers. Net: Layer-2 is the single signal robust across all 3 merge styles — this is the strongest argument yet for prioritizing (2), not just deferring it as "robust follow-up".

Added AC: a merge-commit-style graph case in the bats matrix (prepare commit on the 2nd parent) to pin the --first-parent-vs-full-walk decision by test, so the subtlety can't regress silently.

2. Detection fix and #257 are COMPLEMENTARY, not sequential (corrects the body's "decouple separately")

The body said "decouple-and-fix the coupled CI-trigger gap separately." Sharper framing per Surveyor: fix (1)/(2) and #257 (release-bot PAT so rolling-PR pushes trigger CI) are order-independent + complementary, neither blocks the other:

  • detection fix removes the HARM of commits-on-top (cut fires regardless of what sits above prepare),
  • #257 removes the NEED for the manual ci-trigger commit in the first place.

Either alone improves durability; both is the full close. Not a chain.

3. RETRACT: "insert the ci-trigger commit before the prepare commit"

That suggestion in the body is mechanically wrong — the ci-trigger commit must come after the bot push (the bot push is precisely what fails to trigger CI, so the manual trigger can only follow it). There is no operator-side choreography that avoids the burying. Order-independent detection (this issue) is the only fix. Strike that line from the recommended direction.

Thanks Surveyor — refinement 1 in particular is exactly the cross-merge-style coverage gap fix (2) exists for.

## Folding Surveyor's review refinements (comment 76465) — all three verified, they strengthen the impl ### 1. `--first-parent` is a THIRD merge-style hole (load-bearing — amends fix (1)) Confirmed at source: `release-decide.sh:173` is `git log "${LAST_SHA}..HEAD" --first-parent`. So a fix-(1) range-walk that reuses `COMMITS_RAW` inherits `--first-parent` — and a **merge-commit** merge puts the `prepare` commit on the 2nd parent, invisible to a first-parent walk. So fix (1) as originally worded covers only **rebase / fast-forward** merges (tmux-tell's case), and would pass tmux-tell's rebase tests green while silently leaving **merge-commit** consumers broken. Amended fix (1): the prepare-detection scan must **explicitly choose** `--first-parent` vs a full walk (`git log "${LAST_SHA}..HEAD"` without `--first-parent`, or `git rev-list` over the PR commit set) and pin that choice by test. A full walk closes the merge-commit hole for fix (1); **squash** still loses the subject entirely, which only **fix (2)** (Layer-2 `head.ref == release-prep/rolling` as primary signal) covers. Net: Layer-2 is the single signal robust across all 3 merge styles — this is the strongest argument yet for prioritizing (2), not just deferring it as "robust follow-up". Added AC: **a merge-commit-style graph case in the bats matrix** (prepare commit on the 2nd parent) to pin the `--first-parent`-vs-full-walk decision by test, so the subtlety can't regress silently. ### 2. Detection fix and #257 are COMPLEMENTARY, not sequential (corrects the body's "decouple separately") The body said "decouple-and-fix the coupled CI-trigger gap separately." Sharper framing per Surveyor: fix (1)/(2) and **#257** (release-bot PAT so rolling-PR pushes trigger CI) are **order-independent + complementary**, neither blocks the other: - detection fix removes the **HARM** of commits-on-top (cut fires regardless of what sits above `prepare`), - #257 removes the **NEED** for the manual ci-trigger commit in the first place. Either alone improves durability; both is the full close. Not a chain. ### 3. RETRACT: "insert the ci-trigger commit before the prepare commit" That suggestion in the body is **mechanically wrong** — the ci-trigger commit must come *after* the bot push (the bot push is precisely what fails to trigger CI, so the manual trigger can only follow it). There is no operator-side choreography that avoids the burying. **Order-independent detection (this issue) is the only fix.** Strike that line from the recommended direction. Thanks Surveyor — refinement 1 in particular is exactly the cross-merge-style coverage gap fix (2) exists for.
surveyor removed their assignment 2026-06-29 20:28:30 +02:00
bosun commented 2026-06-29 21:15:06 +02:00
Owner
Copy link

Closed by PR #261 merge at 4283932 — Bosun manually closing since Do=rebase preserved the head commit message as-is (no close-keyword trigger). All ACs verified clean: 6 new bats covering --first-parent / squash / merge-commit matrix + #630 Layer-2-PASS repro + buried+Layer-2-FAIL fail-loud case; Surveyor re-stamp 3275 at rebased head 4283932.

Closed by PR #261 merge at 4283932 — Bosun manually closing since Do=rebase preserved the head commit message as-is (no close-keyword trigger). All ACs verified clean: 6 new bats covering --first-parent / squash / merge-commit matrix + #630 Layer-2-PASS repro + buried+Layer-2-FAIL fail-loud case; Surveyor re-stamp 3275 at rebased head 4283932.
bosun closed this issue 2026-06-29 21:15:06 +02:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
release-prep/rolling
sentry/1465-fragment-contexts
rigger/1463-split-readme-pin-check
i/1426-mirror-propagates-metadata
i/1401-readme-asserts-mirror-state
i/336-sha-pin-propagation
sentry/1342-approval-whitelist-presence
v0.63.1
v0.63.0
v0.62.4
v0.62.3
v0.62.2
v0.62.1
v0.62.0
v0.61.1
v0.61.0
v0.60.0
v0.59.0
v0.58.0
v0.57.4
v0.57.3
v0.57.2
v0.57.1
v0.57.0
v0.56.1
v0.56.0
v0.55.0
v0.54.2
v0.54.1
v0.54.0
v0.53.1
v0.53.0
v0.49.0
v0.52.0
v0.51.0
v0.50.1
v0.50.0
v0.48.2
v0.48.1
v0.48.0
v0.47.0
v0.46.1
v0.46.0
v0.45.1
v0.45.0
v0.44.2
v0.44.1
v0.44.0
v0.43.0
v0.42.0
v0.41.0
v0.40.2
v0.40.1
v0.40.0
v0.39.0
v0.38.0
v0.37.2
v0.37.1
v0.37.0
v0.36.0
v0.35.0
v0.34.0
v1.0.0-alpha.0
v0.33.0
v0.32.0
v0.31.0
v0.30.3
v0.30.2
v0.30.1
v0.30.0
v0.30.0-rc.2
v0.29.0
v0.28.0
v0.27.0
v0.27.0-rc.1
v0.26.0
v0.25.0
v0.24.0
v0.23.0
v0.22.0
v0.21.0-rc.2
v0.21.0-rc.1
v0.21.0
v0.20.0
v0.20.0-rc.2
v0.19.1-rc.1
v0.19.0
v0.19.0-rc.1
v0.18.1
v0.18.0
v0.18.0-rc.1
v0.17.1
v0.17.1-rc.1
v0.17.0
v0.17.0-rc.1
v0.16.0
v0.16.0-rc.1
v0.15.0
v0.15.0-rc.1
v0.14.0
v0.14.0-rc.1
v0.13.1
v0.13.1-rc.1
v0.13.0
v0.13.0-rc.2
v0.12.0
v0.12.0-rc.1
v0.11.1-rc.1
v0.11.0
v0.10.3
v0.10.2
v0.10.1
v0.10.1-rc.1
v0.10.0
v0.9.0
v0.8.0
v0.7.0
v0.6.2
v0.6.1
v0.6.0
v0.5.0
v0.4.0
v0.3.5
v0.3.4
v0.3.3
v0.3.2
v0.3.1
v0.3.0
v0.2.1
v0.2.0
v0.1.0
Labels
Clear labels   
bump
major
 Override the cut bump to MAJOR (release-toolkit#107)

  
bump
minor
 Override the cut bump to MINOR (release-toolkit#107)

  
bump
patch
 Override the cut bump to PATCH (release-toolkit#107)

  
kind/bug
Something is broken

  
kind/chore
Maintenance, tooling, refactor

  
kind/docs
Documentation work

  
kind/feature
New feature or capability

  
priority/critical
Critical: must be addressed immediately

  
priority/high
Important, should be addressed soon

  
priority/low
Low priority, address when convenient

  
priority/medium
Normal priority

  
size/L
Large: significant feature, ~6-16h

  
size/M
Medium: moderate feature, ~2-6h

  
size/S
Small: simple change, ~1-2h

  
size/XL
Extra large: major feature or refactor, 16h+

No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
engineer
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/release-toolkit#259
No description provided.