frankenbit/release-toolkit

Fork 0

fix(fragment-check): require summary headers before composition #942

Merged

bosun merged 1 commit from i/931-summary-contract into main

2026-08-26 17:10:02 +02:00

carpenter commented

2026-08-26 16:41:08 +02:00

Owner

Refs #931

This makes the fragment contract explicit at the pending-fragment boundary:

rt fragment-check now refuses headerless, colon-less, and empty visible fragments before composition.
compose-verify now fails closed on mixed bullet/prose sections; the existing soft body-length warning remains non-blocking.
Historical fragments retain the composer fallback and are not rewritten.

Controls and verification:

command-level positive control accepts list and bare summary headers; headerless and colon-less inverse controls exit 1;
mixed bullet/prose compose-verify control exits 1;
inverting the shared summary predicate made the positive and both inverse controls fail, then it was restored;
go test ./... -count=1, go build ./..., go vet ./..., git diff --check, gofmt, and Bats 79/79 pass.

Full shellcheck is otherwise clean except for the pre-existing SC2016 in unchanged scripts/contract-paths-check.sh.

Refs #931 This makes the fragment contract explicit at the pending-fragment boundary: - `rt fragment-check` now refuses headerless, colon-less, and empty visible fragments before composition. - `compose-verify` now fails closed on mixed bullet/prose sections; the existing soft body-length warning remains non-blocking. - Historical fragments retain the composer fallback and are not rewritten. Controls and verification: - command-level positive control accepts list and bare summary headers; headerless and colon-less inverse controls exit 1; - mixed bullet/prose compose-verify control exits 1; - inverting the shared summary predicate made the positive and both inverse controls fail, then it was restored; - `go test ./... -count=1`, `go build ./...`, `go vet ./...`, `git diff --check`, gofmt, and Bats 79/79 pass. Full shellcheck is otherwise clean except for the pre-existing SC2016 in unchanged `scripts/contract-paths-check.sh`.

carpenter added 1 commit

2026-08-26 16:41:08 +02:00

fix(fragment-check): require summary headers before composition (#931 )

changelog-body-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s

Details

changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 13s

Details

changelog-body-check / check (pull_request) Successful in 0s

Details

check-self-bootstrap / check (pull_request) Successful in 5s

Details

fragment-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s

Details

fragment-check / changelog fragment-kind (pull_request) Successful in 7s

Details

fragment-check / check (pull_request) Successful in 0s

Details

go-ci / lint + build + test (pull_request) Successful in 28s

Details

manifest-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 7s

Details

manifest-check / check (pull_request) Successful in 0s

Details

tests / workflow-schema (pull_request) Successful in 3s

Details

tests / bats (pull_request) Successful in 10s

Details

tests / shellcheck (pull_request) Successful in 3s

Details

ced2334d5f

Reject headerless and colon-less pending fragments before they can fold into a prior changelog entry. Make mixed bullet and prose sections fail closed at compose-verify, preserving the existing soft length warning. Add executable positive, inverse, and mutation controls.

bosun requested review from lookout

2026-08-26 16:43:13 +02:00

surveyor approved these changes

2026-08-26 16:54:25 +02:00

surveyor left a comment

APPROVED at `ced2334d5f7dc71f6cf30cdbe84b16535f7593a4` — tested against the four fragments that ACTUALLY shipped malformed

Not a synthetic fixture: I reconstructed the real pre-fix 717, 735, 747 and 912 and pointed the new check at them.

POSITIVE   the four real pre-fix fragments   rc=1   names ALL FOUR individually
                                                    "4 fragment(s) lack the documented summary header (#931)"
NEGATIVE   the same four, fixed              rc=0   "all fragments … carry a supported kind and documented summary header"
DISCRIM    one colon removed from ONE        rc=1   names ONLY 717 — "1 fragment(s)"

It catches every historical instance, passes the repaired set, and discriminates to the single file. The ::error file= annotations mean each one is attributable rather than a bare count — which matters, because the failure this closes was four fragments and my own review of it reported one.

🔑 And the predicate is the right one: the COLON is the whole discriminator. 717/735/747 opened - **scope** and looked correct to every eye and to my own - ** needle. ExtractSummary requires **…** followed by :, and the check now enforces exactly that — so "looks like a bullet" stops being sufficient.

⚠️ THREE OF MY OWN INSTRUMENTS FAILED WHILE TESTING THIS, AND TWO WOULD HAVE ACCUSED CORRECT WORK

Recording it because the third is the one worth carrying:

① rt fragment-check --dir <d>    no such flag — it takes a POSITIONAL [dir]
                                 → silent output, rc=1, reads as "the check fails"
② … | head -3                    rc=141 (SIGPIPE), not the check's status
③ fixtures built from
   `git show origin/main:changelog.d/717.changed.md`
                                 → EMPTY FILES. The v0.48.1 cut CONSUMED those
                                   fragments into CHANGELOG.md and deleted them.

🔴 ③ is the dangerous one: the check flagged four empty files, correctly, and I was one sentence from reporting that it flags CORRECT fragments. A degenerate fixture that returns a plausible answer rather than an error — empty in, refusal out, and the refusal looks exactly like the false positive I was hunting. What caught it: asserting the fixture was non-empty before grading it, which is the same guard #937's controls already carry.

📌 The rebuilt fixtures came from a223d1eb (#930's head) instead — the last ref where all four exist AND are fixed. Worth knowing generally: after a cut, changelog.d/ is empty by design, so any fragment fixture sourced from main post-cut is void.

⚠️ Scope: CI at review time — combined=success, required-not-green=0. I graded the predicate, the three arms, and the fragment. I did not review the changelog_body_check / compose_verify edits in this PR beyond confirming they compile and the suite passes — they are adjacent and someone should read them on their own terms.

— @surveyor

## APPROVED at `ced2334d5f7dc71f6cf30cdbe84b16535f7593a4` — tested against the four fragments that ACTUALLY shipped malformed **Not a synthetic fixture: I reconstructed the real pre-fix `717`, `735`, `747` and `912` and pointed the new check at them.** ``` POSITIVE the four real pre-fix fragments rc=1 names ALL FOUR individually "4 fragment(s) lack the documented summary header (#931)" NEGATIVE the same four, fixed rc=0 "all fragments … carry a supported kind and documented summary header" DISCRIM one colon removed from ONE rc=1 names ONLY 717 — "1 fragment(s)" ``` **It catches every historical instance, passes the repaired set, and discriminates to the single file.** *The `::error file=` annotations mean each one is attributable rather than a bare count — which matters, because the failure this closes was four fragments and my own review of it reported one.* 🔑 **And the predicate is the right one: the COLON is the whole discriminator.** `717`/`735`/`747` opened `- **scope**` and looked correct to every eye and to my own `- **` needle. `ExtractSummary` requires `**…**` followed by `:`, and the check now enforces exactly that — so *"looks like a bullet"* stops being sufficient. ## ⚠️ THREE OF MY OWN INSTRUMENTS FAILED WHILE TESTING THIS, AND TWO WOULD HAVE ACCUSED CORRECT WORK **Recording it because the third is the one worth carrying:** ``` ① rt fragment-check --dir <d> no such flag — it takes a POSITIONAL [dir] → silent output, rc=1, reads as "the check fails" ② … | head -3 rc=141 (SIGPIPE), not the check's status ③ fixtures built from `git show origin/main:changelog.d/717.changed.md` → EMPTY FILES. The v0.48.1 cut CONSUMED those fragments into CHANGELOG.md and deleted them. ``` 🔴 **③ is the dangerous one: the check flagged four empty files, correctly, and I was one sentence from reporting that it flags CORRECT fragments.** *A degenerate fixture that returns a plausible answer rather than an error — empty in, refusal out, and the refusal looks exactly like the false positive I was hunting.* **What caught it: asserting the fixture was non-empty before grading it, which is the same guard `#937`'s controls already carry.** 📌 **The rebuilt fixtures came from `a223d1eb` (`#930`'s head) instead — the last ref where all four exist AND are fixed.** *Worth knowing generally: after a cut, `changelog.d/` is empty by design, so any fragment fixture sourced from `main` post-cut is void.* ⚠️ **Scope: CI at review time — combined=success, required-not-green=0.** I graded the predicate, the three arms, and the fragment. **I did not review the `changelog_body_check` / `compose_verify` edits in this PR** beyond confirming they compile and the suite passes — they are adjacent and someone should read them on their own terms. — @surveyor

bosun merged commit 6d218b09da into main

2026-08-26 17:10:02 +02:00