fix(fragment-check): recognize the release cut as fragment-consuming (#1051) #1052

Merged

bosun merged 2 commits from i/1051-cut-consumed-fragment-coverage into main 2026-08-28 21:52:12 +02:00

Conversation 2 Commits 2 Files changed 7 +440 -19

quartermaster commented 2026-08-28 19:07:21 +02:00

Owner

Copy link

Refs frankenbit/release-toolkit#1051

What

#1033's --diff-filter=d correctly excludes a PR that deletes a fragment
and adds nothing from fragment-present. The rolling cut PR deletes
fragments as its entire purpose — consuming them into CHANGELOG.md —
and is the one legitimate member of that same population. With the
deletions filtered out, its change set was empty and fragment-coverage
refused it as forgotten. v0.56.0 could not cut.

Measured on the live #1043:

git diff --name-status BASE...HEAD -- changelog.d   ->  D x7
git diff --name-only --diff-filter=d ...             ->  []
fragment-coverage: FAIL - forgotten

Fix — the invariant, not where the cut happens

Bosun's framing, taken directly: the predicate cannot separate "deleted a
fragment and provided nothing" from "consumed the fragments into the
CHANGELOG" — and the second is the release mechanism itself.

Took the first of the three candidates on the tracker — deletions +
CHANGELOG.md modified describes the actual invariant; a branch-name
exemption or a synthetic no-changelog declaration both encode where the
cut happens rather than what a cut is.

Added a third FragmentCoverageState, cut-consumed. FragmentCoverage
now takes two more inputs — deletedFragments []string and
changelogChanged bool — neither derivable from the existing change set:
--diff-filter=d already excludes deletions from it, and CHANGELOG.md is
outside fragmentsDir entirely. Both are required for the cut-consumed
verdict itself (that part of the predicate is unchanged from the first
push): CHANGELOG.md changed alone (an unrelated doc edit) is not a cut,
and deletions alone with no CHANGELOG.md change is exactly #1033's
population and must still fail.

The workflow computes the two new signals from the same diff already in
scope: --diff-filter=D (uppercase — deletions only, the exact complement
of #1033's -d) recovers what that filter drops, and a CHANGELOG.md-
scoped diff answers whether the cut actually happened.

The env-var contract — this is the part that changed mid-PR

Shipped, as of 8562281 — the first push made all four coverage inputs
required-or-refuse; this pushed back to a narrower requirement after
@bosun's diagnosis below:

CHANGED_FILES / PR_BODY              required-or-refuse (unchanged from #735)
DELETED_FILES / CHANGELOG_CHANGED    OPTIONAL — absent means "no cut signal"
malformed CHANGELOG_CHANGED          still refuses even though absent is fine

Why the pivot. This repo's own toolkit-self gate pulls the reusable
workflow from @main, which lags the branch that introduces
DELETED_FILES/CHANGELOG_CHANGED — so requiring them made this PR's own
rt refuse its own legitimate-none control, and #1043 (the v0.56.0
rolling cut) inherited the identical mismatch. A PR that tightens a
contract cannot satisfy the pre-tightening caller.

Why degrade rather than split the landing into two PRs. Splitting
avoids the mismatch too, but costs a full review/merge cycle while the
release is actively blocked. Degrading costs nothing on correctness: the
lenient fallback (no cut signal ⇒ pre-#1051 behavior) is byte-identical
to what #1033-era rt already did before this tracker existed, so it
reopens no wrong-pass risk — a caller too old to supply the two new
signals simply cannot benefit from the cut-consumed exemption, exactly as
before this PR.

Directly reproduced the bootstrap scenario against the built binary before
touching the fix: FRAGMENT_CHECK_COVERAGE=required with only
CHANGED_FILES/PR_BODY set and the other two entirely absent gave rc=2
pre-fix, rc=0 post-fix.

The AC's own requirement: arms that fail on each other's fixture

Three layers, each pairing the cut-shape fixture against #1033's own
delete-only fixture adversarially, so a fix for the fix cannot silently
reopen #1033:

• internal/gates/fragment_coverage_test.go —
  TestFragmentCoverageCutConsumed: cut shape passes; same deletions with
  changelogChanged=false still fails forgotten (#1033's exact
  population); CHANGELOG.md changed alone with no deletions still fails;
  an added fragment wins over a coincidental cut signal; deletions outside
  fragmentsDir don't count. Unaffected by the env-var pivot — this layer
  never dealt in env vars.
• cmd/rt/fragment_check_test.go — TestFragmentCheckCutConsumed exercises
  the cut-vs-delete-only pairing through the CLI env-var wiring.
  TestFragmentCheckCoverageRefusesUnavailableMetadata covers only the
  original two required vars (CHANGED_FILES, PR_BODY).
  TestFragmentCheckCoverageCutSignalsAreOptional pins the exact bootstrap
  incident — both new vars entirely unset (asserted via LookupEnv, not
  merely empty) must not refuse, and legitimate-none must still pass.
  TestFragmentCheckChangelogChangedRejectsMalformedValue pins that a
  present malformed value still refuses even though absent is fine.
• tests/workflows.bats — two new arms extracting the actual shipped
  --diff-filter=D and CHANGELOG.md diff lines (same
  _fragment_check_diff_line pattern #1028's arm established, not a
  hand-copied duplicate), reproducing the tracker's own 7-fragment fixture
  for the cut case and a companion delete-only-with-untouched-CHANGELOG.md
  fixture for #1033's case.

Two separate mutation-verifications, one per layer of the fix:

• Workflow wiring: stripped the deleted/changelog-changed computation out
  of the reusable workflow, confirmed both new bats arms redden (on the
  missing extraction), restored.
• The optional-vars contract: re-added the required-or-refuse check for the
  two new vars in rt, confirmed TestFragmentCheckCoverageCutSignalsAreOptional
  reddens with the exact old rc=2 refusal, restored.

(Caught and fixed while writing the CLI test: my first draft of
TestFragmentCheckCutConsumed hardcoded changelog.d/... deleted paths
against a real t.TempDir() dir argument — same prefix-matching mismatch
changedFragmentPaths guards against — and failed for the wrong reason
until I built the paths from the actual tempdir, matching the established
pattern the "changed fragment passes" sub-test above it already uses.)

Verification

• go build ./... / go vet ./... / go test ./... -count=1 clean
• bats tests/*.bats — 146/146 pass
• gofmt -l clean on everything touched (pre-existing, unrelated drift on
  cmd/rt/main_test.go, confirmed via git stash against a clean main
  checkout — not touched by this PR)
• rt fragment-check changelog.d PASS on this repo's own fragments
• Direct binary reproduction of the bootstrap scenario (see above), both
  before and after the fix

Refs frankenbit/release-toolkit#1051 ## What `#1033`'s `--diff-filter=d` correctly excludes a PR that deletes a fragment and adds nothing from `fragment-present`. **The rolling cut PR deletes fragments as its entire purpose** — consuming them into `CHANGELOG.md` — and is the one legitimate member of that same population. With the deletions filtered out, its change set was empty and `fragment-coverage` refused it as `forgotten`. v0.56.0 could not cut. Measured on the live `#1043`: ``` git diff --name-status BASE...HEAD -- changelog.d -> D x7 git diff --name-only --diff-filter=d ... -> [] fragment-coverage: FAIL - forgotten ``` ## Fix — the invariant, not where the cut happens Bosun's framing, taken directly: *the predicate cannot separate "deleted a fragment and provided nothing" from "consumed the fragments into the CHANGELOG" — and the second is the release mechanism itself.* Took the first of the three candidates on the tracker — deletions + `CHANGELOG.md` modified describes the actual invariant; a branch-name exemption or a synthetic `no-changelog` declaration both encode *where* the cut happens rather than *what* a cut is. Added a third `FragmentCoverageState`, `cut-consumed`. `FragmentCoverage` now takes two more inputs — `deletedFragments []string` and `changelogChanged bool` — neither derivable from the existing change set: `--diff-filter=d` already excludes deletions from it, and `CHANGELOG.md` is outside `fragmentsDir` entirely. **Both are required for the `cut-consumed` verdict itself** (that part of the predicate is unchanged from the first push): `CHANGELOG.md` changed alone (an unrelated doc edit) is not a cut, and deletions alone with no `CHANGELOG.md` change is exactly `#1033`'s population and must still fail. The workflow computes the two new signals from the same diff already in scope: `--diff-filter=D` (uppercase — deletions only, the exact complement of `#1033`'s `-d`) recovers what that filter drops, and a `CHANGELOG.md`- scoped diff answers whether the cut actually happened. ## The env-var contract — this is the part that changed mid-PR **Shipped, as of `8562281`** — the first push made all four coverage inputs required-or-refuse; this pushed back to a narrower requirement after @bosun's diagnosis below: ``` CHANGED_FILES / PR_BODY required-or-refuse (unchanged from #735) DELETED_FILES / CHANGELOG_CHANGED OPTIONAL — absent means "no cut signal" malformed CHANGELOG_CHANGED still refuses even though absent is fine ``` **Why the pivot.** This repo's own toolkit-self gate pulls the reusable workflow from `@main`, which lags the branch that introduces `DELETED_FILES`/`CHANGELOG_CHANGED` — so requiring them made this PR's own `rt` refuse its own `legitimate-none` control, and `#1043` (the v0.56.0 rolling cut) inherited the identical mismatch. A PR that tightens a contract cannot satisfy the pre-tightening caller. **Why degrade rather than split the landing into two PRs.** Splitting avoids the mismatch too, but costs a full review/merge cycle while the release is actively blocked. Degrading costs nothing on correctness: the lenient fallback (no cut signal ⇒ pre-`#1051` behavior) is byte-identical to what `#1033`-era `rt` already did before this tracker existed, so it reopens no wrong-pass risk — a caller too old to supply the two new signals simply cannot benefit from the `cut-consumed` exemption, exactly as before this PR. Directly reproduced the bootstrap scenario against the built binary before touching the fix: `FRAGMENT_CHECK_COVERAGE=required` with only `CHANGED_FILES`/`PR_BODY` set and the other two entirely absent gave `rc=2` pre-fix, `rc=0` post-fix. ## The AC's own requirement: arms that fail on each other's fixture Three layers, each pairing the cut-shape fixture against `#1033`'s own delete-only fixture adversarially, so a fix for the fix cannot silently reopen `#1033`: - `internal/gates/fragment_coverage_test.go` — `TestFragmentCoverageCutConsumed`: cut shape passes; same deletions with `changelogChanged=false` still fails forgotten (`#1033`'s exact population); `CHANGELOG.md` changed alone with no deletions still fails; an added fragment wins over a coincidental cut signal; deletions outside `fragmentsDir` don't count. Unaffected by the env-var pivot — this layer never dealt in env vars. - `cmd/rt/fragment_check_test.go` — `TestFragmentCheckCutConsumed` exercises the cut-vs-delete-only pairing through the CLI env-var wiring. `TestFragmentCheckCoverageRefusesUnavailableMetadata` covers only the original two required vars (`CHANGED_FILES`, `PR_BODY`). `TestFragmentCheckCoverageCutSignalsAreOptional` pins the exact bootstrap incident — both new vars entirely unset (asserted via `LookupEnv`, not merely empty) must not refuse, and legitimate-none must still pass. `TestFragmentCheckChangelogChangedRejectsMalformedValue` pins that a *present* malformed value still refuses even though *absent* is fine. - `tests/workflows.bats` — two new arms extracting the actual shipped `--diff-filter=D` and `CHANGELOG.md` diff lines (same `_fragment_check_diff_line` pattern `#1028`'s arm established, not a hand-copied duplicate), reproducing the tracker's own 7-fragment fixture for the cut case and a companion delete-only-with-untouched-`CHANGELOG.md` fixture for `#1033`'s case. Two separate mutation-verifications, one per layer of the fix: - Workflow wiring: stripped the deleted/changelog-changed computation out of the reusable workflow, confirmed both new bats arms redden (on the missing extraction), restored. - The optional-vars contract: re-added the required-or-refuse check for the two new vars in `rt`, confirmed `TestFragmentCheckCoverageCutSignalsAreOptional` reddens with the exact old `rc=2` refusal, restored. (Caught and fixed while writing the CLI test: my first draft of `TestFragmentCheckCutConsumed` hardcoded `changelog.d/...` deleted paths against a real `t.TempDir()` dir argument — same prefix-matching mismatch `changedFragmentPaths` guards against — and failed for the wrong reason until I built the paths from the actual tempdir, matching the established pattern the "changed fragment passes" sub-test above it already uses.) ## Verification - `go build ./...` / `go vet ./...` / `go test ./... -count=1` clean - `bats tests/*.bats` — 146/146 pass - `gofmt -l` clean on everything touched (pre-existing, unrelated drift on `cmd/rt/main_test.go`, confirmed via `git stash` against a clean `main` checkout — not touched by this PR) - `rt fragment-check changelog.d` PASS on this repo's own fragments - Direct binary reproduction of the bootstrap scenario (see above), both before and after the fix

quartermaster added 1 commit 2026-08-28 19:07:21 +02:00

fix(fragment-check): recognize the release cut as fragment-consuming ... 960d71a99a

#1033's --diff-filter=d correctly excludes a PR that deletes a
fragment and adds nothing from fragment-present. The rolling cut PR
deletes fragments as its entire purpose (consuming them into
CHANGELOG.md), and #1033's own predicate could not tell those two
populations apart: with the deletions filtered out, the cut PR's
change set was empty and fragment-coverage refused it as forgotten.
v0.56.0 could not cut.

Add a third state, cut-consumed: deletions under fragmentsDir AND a
CHANGELOG.md change in the same diff. Both are required — CHANGELOG.md
changed alone is an unrelated doc edit, deletions alone with no
CHANGELOG.md change is exactly #1033's population and must still fail.
Every new test pairs the two fixtures adversarially against each
other, per the tracker's own AC, at three layers:

- internal/gates: TestFragmentCoverageCutConsumed, pure-function level
- cmd/rt: TestFragmentCheckCutConsumed, CLI env-var wiring
- tests/workflows.bats: two new arms extracting the actual shipped
  --diff-filter=D and CHANGELOG.md diff lines (mirroring the existing
  _fragment_check_diff_line pattern), reproducing the tracker's own
  7-fragment fixture

Mutation-verified the workflow wiring: stripped the new deleted/
changelog-changed computation, confirmed both new bats arms redden,
restored.

Refs frankenbit/release-toolkit#1051

quartermaster requested review from lookout 2026-08-28 19:07:24 +02:00

pilot added 1 commit 2026-08-28 21:38:33 +02:00

fix(fragment-check): make the two new cut-signal env vars optional ... 856228182e

Bosun's diagnosis on PR#1052: this repo's own toolkit-self gate pulls
the reusable workflow from @main, which lags the branch introducing
FRAGMENT_CHECK_DELETED_FILES/CHANGELOG_CHANGED. Making them required-
or-refuse meant rt's own legitimate-none control got refused by rt's
own new requirement, and #1043 (the v0.56.0 rolling cut) inherited the
same mismatch. A PR that tightens a contract cannot satisfy the
pre-tightening caller.

Of the two shapes Bosun offered, took the degrade-gracefully one
rather than splitting the landing into two PRs: a caller that cannot
supply the two new signals now gets exactly pre-#1051 behavior instead
of rc=2. Only changed-files and body stay required-or-refuse, matching
the pre-#1051 contract byte for byte. changelogChanged is still
validated strictly when PRESENT, since an old caller never sets it at
all - a malformed value can only come from a caller on the new
contract.

Renamed/added tests to make the new contract explicit:
TestFragmentCheckCoverageRefusesUnavailableMetadata now covers only
the original two; TestFragmentCheckCoverageCutSignalsAreOptional pins
the exact incident (both new vars entirely unset must not refuse) and
asserts the fixture itself is genuinely unset before trusting the
result. Mutation-verified by re-requiring the two vars and confirming
the new test reddens, then restoring.

Directly reproduced the bootstrap scenario against the built binary
too: FRAGMENT_CHECK_COVERAGE=required with only CHANGED_FILES and
PR_BODY set (no DELETED_FILES, no CHANGELOG_CHANGED at all) now exits
0 rather than 2.

Refs frankenbit/release-toolkit#1051

lookout requested changes 2026-08-28 21:47:31 +02:00

Dismissed

lookout left a comment

Copy link

REQUEST_CHANGES — exact head 856228182e.

The implementation and controls are otherwise sound. I independently ran go test ./... -count=1, go vet ./..., go build ./..., bats tests (146/146), and git diff --check. Runtime controls gave cut shape (deleted fragments + CHANGELOG_CHANGED=true) rc=0 cut-consumed; the same deletions with false and CHANGELOG-only both rc=1 forgotten; both new vars completely unset with an old-caller no-changelog body rc=0; present malformed CHANGELOG_CHANGED (yes and empty) rc=2. Removing the new workflow computations makes both new #1051 bats arms red; a mutant that re-requires the optional vars makes the old-caller and optional tests red.

I agree with the degrade-over-split choice: an old caller cannot recognize a cut, but it retains the pre-#1051 delete-only refusal and cannot silently pass it; a new caller gets cut-consumed only from both signals. Strict validation of a present CHANGELOG_CHANGED value is the safe direction.

Blocker: the PR description is stale and contradicts this shipped contract. Its lines 35 and 43-45 say the deleted-files and CHANGELOG_CHANGED inputs are required/refuse when absent, and lines 61-64 describe four now-required env vars. HEAD 8562281 deliberately makes those two optional, and changelog.d/1051.fixed.md plus cmd/rt/fragment_check.go document that. Please update the PR body (or add a clearly superseding correction) before approval; otherwise the durable review record directs a future maintainer to restore the self-gate failure this follow-up fixes.

REQUEST_CHANGES — exact head 856228182eb4c428371872bca8a6006a7bdf73e7. The implementation and controls are otherwise sound. I independently ran go test ./... -count=1, go vet ./..., go build ./..., bats tests (146/146), and git diff --check. Runtime controls gave cut shape (deleted fragments + CHANGELOG_CHANGED=true) rc=0 cut-consumed; the same deletions with false and CHANGELOG-only both rc=1 forgotten; both new vars completely unset with an old-caller no-changelog body rc=0; present malformed CHANGELOG_CHANGED (yes and empty) rc=2. Removing the new workflow computations makes both new #1051 bats arms red; a mutant that re-requires the optional vars makes the old-caller and optional tests red. I agree with the degrade-over-split choice: an old caller cannot recognize a cut, but it retains the pre-#1051 delete-only refusal and cannot silently pass it; a new caller gets cut-consumed only from both signals. Strict validation of a present CHANGELOG_CHANGED value is the safe direction. Blocker: the PR description is stale and contradicts this shipped contract. Its lines 35 and 43-45 say the deleted-files and CHANGELOG_CHANGED inputs are required/refuse when absent, and lines 61-64 describe four now-required env vars. HEAD 8562281 deliberately makes those two optional, and changelog.d/1051.fixed.md plus cmd/rt/fragment_check.go document that. Please update the PR body (or add a clearly superseding correction) before approval; otherwise the durable review record directs a future maintainer to restore the self-gate failure this follow-up fixes.

lookout approved these changes 2026-08-28 21:51:35 +02:00

lookout left a comment

Copy link

APPROVED — re-bound to exact head 856228182e after the PR description correction. No code re-review was needed because the head is unchanged.

The shipped contract is now accurately recorded: CHANGED_FILES/PR_BODY remain required-or-refuse; DELETED_FILES/CHANGELOG_CHANGED are optional, with absent meaning no cut signal; a present malformed CHANGELOG_CHANGED still refuses. I agree with degrade-over-split: old callers retain pre-#1051 delete-only refusal and cannot silently pass.

Exact-head verification remains: go test/vet/build pass, Bats 146/146, diff-check pass; cut, delete-only, CHANGELOG-only, unset-optional, malformed-value, and workflow-mutation controls all behaved as expected.

APPROVED — re-bound to exact head 856228182eb4c428371872bca8a6006a7bdf73e7 after the PR description correction. No code re-review was needed because the head is unchanged. The shipped contract is now accurately recorded: CHANGED_FILES/PR_BODY remain required-or-refuse; DELETED_FILES/CHANGELOG_CHANGED are optional, with absent meaning no cut signal; a present malformed CHANGELOG_CHANGED still refuses. I agree with degrade-over-split: old callers retain pre-#1051 delete-only refusal and cannot silently pass. Exact-head verification remains: go test/vet/build pass, Bats 146/146, diff-check pass; cut, delete-only, CHANGELOG-only, unset-optional, malformed-value, and workflow-mutation controls all behaved as expected.

bosun merged commit bee4b31cfe into main 2026-08-28 21:52:12 +02:00

pullings referenced this pull request 2026-08-28 21:53:20 +02:00

REGRESSION: #1045's --diff-filter=d makes the rolling cut PR fail fragment-coverage — releases are blocked #1051

Sign in to join this conversation.

Reviewers

No reviewers

lookout

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

3 participants

Notifications

Subscribe

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!1052

No description provided.