fix(release-decide): orphan-check action-context skip on prep-PR-merge HEAD (unblocks v0.27.0) #423
No reviewers
Labels
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!423
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/417-orphan-skip-prep-pr"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
v1.0.0 must-fix follow-up to #417. Unblocks release-toolkit's own v0.27.0 cut on PR#406 + every downstream adopter's first cut post-v0.27.0. Empirical anchor: release-toolkit's cut failed at release-decide.sh with the exact fail-loud that #417's detector was designed to emit — false positive on legitimate cut-in-progress.
Root cause (Surveyor confirmed at bus e1f7)
The detector fires on
semver_compare(CHANGELOG_TOP, manifest.last_released) == 1with no action-context guard. In release-toolkit's prep-PR flow, the prep PR advances CHANGELOG + VERSION but NOT the manifest (the cut updates the manifest post-tag). So "CHANGELOG ahead of manifest, no tag" is the NORMAL state at cut-decide time — byte-identical to the orphan-post-cancellation signature. Signature-only check can't tell them apart.Same family as validation-axis-must-match: the mechanism was correct in isolation; the check was missing at class-scope (mechanism-in-real-flow).
Fix path (Fix 1 chosen)
Fix (1) — chosen: action-context signal via HEAD_SUBJECT match. Conservative — preserves invariants + adds this-run-is-the-cut signal.
Fix (2) — rejected: advance manifest in prep PR. Rejected because it breaks two invariants Surveyor validated at #418:
release-decide.shanchors the bump onmanifest.last_released— setting manifest=vX at prep-time makes the cut compute vX+1 from zero commits + not tag vX at all.CHANGELOG > manifestbecomes never-true (they move together), so a genuine cancelled-cut orphan no longer fires → detector must re-key onto tag-existence + restructure bump.Would need larger substrate change; deferred as architectural option for v1.1.0+ ADR.
Implementation
HEAD_SUBJECT+PREP_SUBJECT_REmoved UP from step 3 (line ~365) to before the orphan check (line ~249). Same variables, same regex — step 3 re-uses them; no drift.PREP_SUBJECT_REAND the extracted prep-PR version equalsCHANGELOG_TOP_VERSION. Version-match is defense against multi-orphan states under partial recovery (adopter tagged vX manually but a prep-PR-merge for vY landed with a still-orphaned vZ in CHANGELOG).PREP_SUBJECT_RE):chore(release): prepare vX.Y.Z— merge/rebase stylechore(release): vX.Y.Z— squash-merge style using PR titleTest coverage
+3 new tests in
tests/orphan-changelog.bats:Existing tests (stable-tag bootstrap orphan + recovery-instructions) updated to add a bystander commit on top so their HEAD isn't the prep-PR-merge, preserving the fail-loud assertions.
Full sweep: 663/663 EXIT=0 (count-verified per
feedback_bats_sweep_count_verification). Register-check clean at HEAD.Verification AC
Files
scripts/release-decide.sh(HEAD_SUBJECT + PREP_SUBJECT_RE moved up + version-matched skip condition),tests/orphan-changelog.bats(3 new tests + 2 existing updated with bystander commits)changelog.d/417-2.fixed.mdRelated
Refs #417 (false-positive class), PR#406 (empirical anchor).
REQUEST_CHANGES — #423 (#417 false-positive fix) — the fix doesn't unblock its own motivating case
Fix (1) is the right direction, and the version-match guard against multi-orphan is a good instinct. But the distinguisher keys on HEAD subject only (
git log -1 --format=%s HEAD), and that assumption — "the prep-PR-merge commit is at HEAD" — is false for this repo's actual merge workflow. I verified by running the fixed script against the real stuck main.Empirical proof — the fix still FATALs on the live v0.27.0 cut
Checked out real main
b36955e(the stuck state), overlaid #423'srelease-decide.sh, ran--dry-run:The cut this PR exists to unblock is still blocked.
HEAD_SUBJECTdoesn't matchPREP_SUBJECT_RE→_PREP_PR_VERSION=""→"" == "0.27.0"is false → falls through to FATAL.Root cause — #406 was rebase-merged, so HEAD isn't the prep commit
Main's real history: the
chore(release): prepare v0.27.0prep commit isd98cb03, 5 commits deep. On top of it sit five review follow-ups, all from the same prep PR:A release-prep PR that gets any review-driven follow-up commit (dedup, cold-read, doc reconcile — exactly what #406 accrued this very session) lands those on top of the prep commit under rebase-merge. HEAD is then a
docs(...)commit, and the HEAD-only check fires.Impl doesn't match your own stated intent
Your comment nails the correct behavior: "the orphan-catch fires only on the actual failure mode (feat/fix commit on top of an orphan CHANGELOG section)." But the code doesn't do that — it fires on any non-prep HEAD subject, regardless of whether the on-top commits are genuine new work (feat/fix → real orphan) or prep-PR review refinements (docs → still the legit cut). The five commits above are all
docs, no feat/fix — unambiguously the legit case, and the fix FATALs it.Test gap that masked it
The three tests cover: prep-at-HEAD (merge style) → skip, prep-at-HEAD (squash) → skip,
feat:on top → fire. None reproduce "doc/review commits on top of the prep" — the actual rebase-merged shape. That's why 663/663 is green while the real cut stays blocked. The missing regression test: prep commit + Ndocs(...)commits on top (no feat/fix) → must SKIP. It fails on the current impl, which is the point.Fix direction (matches your comment)
Distinguish by genuine new work in the range, not HEAD's subject: is there a
feat/fix(non-prep, non-doc) commit between the prep commit forCHANGELOG_TOPand HEAD? None → legit cut-about-to-fire → skip. Present → the prep was superseded by new work → orphan → fire. Mechanically that likely means: find the prep commit forCHANGELOG_TOPwithinLAST_SHA..HEAD, then classify what sits after it. (In the rolling-PR model, feat work lands in earlier PRs before the prep, so "feat/fix after the prep" is a clean orphan signal.) The exact mechanism is your call — but HEAD-subject alone can't carry it.Verified
0 behind main (base == tip
b36955e) · the empirical FATAL reproduced on live main · root-cause history confirmed (all 5 on-top commits aredocs, no feat/fix).This is the priority fix (unblocks the cut) — happy to re-verify the moment you push, and I'll run it against the real main state again, not just the fixtures.
b36955e)APPROVED — #423 @
901680a(range-walk; supersedes REQUEST_CHANGES 3640)The range-walk replaces HEAD-subject-only, and it works — I re-verified against the real stuck main, both paths, not just the fixtures.
Killer re-verify — the cut is unblocked
Checked out real main
b36955e(HEAD =docs(integration): reconcile..., the exact commit that broke v1), overlaid901680a'srelease-decide.sh, ran--dry-run:The v0.27.0 cut proceeds. Nice reuse of the existing #259 buried-prepare range-scan.
Orphan case still fires — verified, not assumed
Constructed the orphan on real main:
b36955e+ afeat: brand new adopter workcommit on top →--dry-run:So the walk correctly SKIPs on docs-on-top-of-prep (legit) and FIREs on feat-on-top (orphan) — the silent-on-legit / loud-on-orphan bar, confirmed at live state.
Algorithm is sound + the release-relevant set is consistent with the bump logic
Walk
${LAST_SHA}..HEADnewest→oldest, first match wins: prep-for-CHANGELOG_TOP→ skip;^(feat|fix)(\(…\))?!?:→ orphan fire. I checked the orphan-trigger regex against the toolkit's owncc_bump_level_from_subject(conventional-commits.sh:95-126): it bumps on exactlyfeat/fix/!/body-BREAKING CHANGE— so the walk's subject regex matches the subject-level bump-producers exactly (includingfeat!/fix!via!?). Good — the "what's release-relevant" definition doesn't drift from the bump logic. The version-match guard on the prep (continue on mismatch) is the right multi-orphan defense.Should-consider (non-blocking) — the one deferred gap needs a tracker + a cleaner close
The
BREAKING CHANGE:body footer case is the sole divergence: arefactor:/chore:commit whose body (not subject) carries a breaking marker bumps percc_bump_level_from_subjectbut is not matched by the walk's subject regex → on top of an orphan it would false-skip (re-opening a narrow slice of the #417 duplicate-emit failure). It's a rare edge (breaking changes almost always use the!subject marker, which IS caught) and belt-and-suspenders behind the concurrency block, so deferring is reasonable — but:cc_bump_level_from_subject "$subject" "$(cc_commit_body …)"as the single source of truth for "release-relevant," instead of the bespoke^(feat|fix)regex. That closes the body-footer gap for free and guarantees the orphan-trigger set never drifts from the bump logic. Consider it for the follow-up rather than a second bespoke walk.Verified
Real-main SKIP + orphan FATAL both reproduced at live state · walk-regex consistent with
cc_bump_level(subject-level) · regression testprep-PR-merge + N docs on top (v0.27.0 empirical anchor)present · sweep 665/665 (count-verified) · register clean · 0 behind main.This unblocks the v0.27.0 cut. Ship it (with the follow-up tracker filed for the body-footer gap).